Handle network and permissions errors when saving group member label.

This commit is contained in:
jeffrey-signal
2026-02-26 10:34:16 -05:00
committed by GitHub
parent 316d0e67c5
commit 9581994050
7 changed files with 119 additions and 29 deletions

View File

@@ -863,7 +863,7 @@ class ConversationSettingsFragment :
navController.safeNavigate(action)
},
onDisabledClicked = {
Snackbar.make(requireView(), R.string.ConversationSettingsFragment__only_admins_can_add_member_labels, Snackbar.LENGTH_SHORT).show()
Snackbar.make(requireView(), R.string.GroupMemberLabel__error_no_edit_permission, Snackbar.LENGTH_SHORT).show()
}
)
}

View File

@@ -2,7 +2,7 @@ package org.thoughtcrime.securesms.groups;
public final class GroupInsufficientRightsException extends GroupChangeException {
GroupInsufficientRightsException(Throwable throwable) {
public GroupInsufficientRightsException(Throwable throwable) {
super(throwable);
}
}

View File

@@ -21,6 +21,8 @@ import androidx.compose.foundation.verticalScroll
import androidx.compose.material3.Icon
import androidx.compose.material3.IconButton
import androidx.compose.material3.MaterialTheme
import androidx.compose.material3.SnackbarHost
import androidx.compose.material3.SnackbarHostState
import androidx.compose.material3.Text
import androidx.compose.material3.TextFieldDefaults
import androidx.compose.runtime.Composable
@@ -42,6 +44,7 @@ import androidx.core.os.bundleOf
import androidx.lifecycle.compose.collectAsStateWithLifecycle
import org.signal.core.ui.compose.AllDevicePreviews
import org.signal.core.ui.compose.Buttons
import org.signal.core.ui.compose.CircularProgressWrapper
import org.signal.core.ui.compose.ClearableTextField
import org.signal.core.ui.compose.ComposeFragment
import org.signal.core.ui.compose.Previews
@@ -87,6 +90,7 @@ class MemberLabelFragment : ComposeFragment(), ReactWithAnyEmojiBottomSheetDialo
override fun FragmentContent() {
val uiState by viewModel.uiState.collectAsStateWithLifecycle()
val backPressedDispatcher = LocalOnBackPressedDispatcherOwner.current?.onBackPressedDispatcher
val snackbarHostState = remember { SnackbarHostState() }
val callbacks = remember {
object : MemberLabelUiCallbacks {
@@ -102,16 +106,34 @@ class MemberLabelFragment : ComposeFragment(), ReactWithAnyEmojiBottomSheetDialo
}
}
val networkErrorMessage = stringResource(R.string.GroupMemberLabel__error_cant_save_no_network)
val noPermissionErrorMessage = stringResource(R.string.GroupMemberLabel__error_no_edit_permission)
LaunchedEffect(uiState.saveState) {
if (uiState.saveState is SaveState.Success) {
backPressedDispatcher?.onBackPressed()
viewModel.onSaveStateConsumed()
when (uiState.saveState) {
is SaveState.Success -> {
backPressedDispatcher?.onBackPressed()
viewModel.onSaveStateConsumed()
}
is SaveState.NetworkError -> {
snackbarHostState.showSnackbar(networkErrorMessage)
viewModel.onSaveStateConsumed()
}
is SaveState.InsufficientRights -> {
snackbarHostState.showSnackbar(noPermissionErrorMessage)
viewModel.onSaveStateConsumed()
}
is SaveState.InProgress, null -> Unit
}
}
MemberLabelScreenUi(
state = uiState,
callbacks = callbacks
callbacks = callbacks,
snackbarHostState = snackbarHostState
)
}
@@ -127,13 +149,15 @@ class MemberLabelFragment : ComposeFragment(), ReactWithAnyEmojiBottomSheetDialo
@Composable
private fun MemberLabelScreenUi(
state: MemberLabelUiState,
callbacks: MemberLabelUiCallbacks
callbacks: MemberLabelUiCallbacks,
snackbarHostState: SnackbarHostState = remember { SnackbarHostState() }
) {
Scaffolds.Settings(
title = stringResource(R.string.GroupMemberLabel__title),
onNavigationClick = callbacks::onClosePressed,
navigationIcon = SignalIcons.X.imageVector,
navigationContentDescription = stringResource(R.string.GroupMemberLabel__accessibility_close_screen)
navigationContentDescription = stringResource(R.string.GroupMemberLabel__accessibility_close_screen),
snackbarHost = { SnackbarHost(snackbarHostState) }
) { paddingValues ->
val focusRequester = remember { FocusRequester() }
@@ -191,13 +215,17 @@ private fun MemberLabelScreenUi(
}
}
SaveButton(
enabled = state.isSaveEnabled,
onClick = callbacks::onSaveClicked,
CircularProgressWrapper(
isLoading = state.saveState is SaveState.InProgress,
modifier = Modifier
.align(Alignment.BottomEnd)
.padding(end = 24.dp, bottom = 16.dp)
)
) {
SaveButton(
enabled = state.isSaveEnabled,
onClick = callbacks::onSaveClicked
)
}
}
}
}

View File

@@ -22,6 +22,7 @@ import org.thoughtcrime.securesms.groups.GroupManager
import org.thoughtcrime.securesms.recipients.Recipient
import org.thoughtcrime.securesms.recipients.RecipientId
import org.thoughtcrime.securesms.util.RemoteConfig
import org.whispersystems.signalservice.api.NetworkResult
/**
* Handles the retrieval and modification of group member labels.
@@ -52,12 +53,6 @@ class MemberLabelRepository private constructor(
@WorkerThread
fun getLabelJava(groupId: GroupId.V2, recipient: Recipient): MemberLabel? = runBlocking { getLabel(groupId, recipient) }
/**
* Checks whether the [Recipient] has permission to set their member label in the given group (blocking version for Java compatibility).
*/
@WorkerThread
fun canSetLabelJava(groupId: GroupId.V2, recipient: Recipient): Boolean = runBlocking { canSetLabel(groupId, recipient) }
/**
* Gets the member label for a specific recipient in the group.
*/
@@ -117,13 +112,15 @@ class MemberLabelRepository private constructor(
/**
* Sets the group member label for the current user.
*/
suspend fun setLabel(groupId: GroupId.V2, label: MemberLabel): Unit = withContext(Dispatchers.IO) {
suspend fun setLabel(groupId: GroupId.V2, label: MemberLabel): NetworkResult<Unit> = withContext(Dispatchers.IO) {
if (!RemoteConfig.sendMemberLabels) {
throw IllegalStateException("Set member label not allowed due to remote config.")
}
val sanitizedLabel = label.sanitized()
GroupManager.updateMemberLabel(context, groupId, sanitizedLabel.text, sanitizedLabel.emoji.orEmpty())
NetworkResult.fromFetch {
GroupManager.updateMemberLabel(context, groupId, sanitizedLabel.text, sanitizedLabel.emoji.orEmpty())
}
}
}

View File

@@ -16,9 +16,11 @@ import org.signal.core.util.StringUtil
import org.signal.core.util.concurrent.SignalDispatchers
import org.thoughtcrime.securesms.conversation.colors.NameColor
import org.thoughtcrime.securesms.groups.GroupId
import org.thoughtcrime.securesms.groups.GroupInsufficientRightsException
import org.thoughtcrime.securesms.groups.memberlabel.MemberLabelUiState.SaveState
import org.thoughtcrime.securesms.recipients.Recipient
import org.thoughtcrime.securesms.recipients.RecipientId
import org.whispersystems.signalservice.api.NetworkResult
class MemberLabelViewModel(
private val memberLabelRepo: MemberLabelRepository = MemberLabelRepository.instance,
@@ -97,7 +99,7 @@ class MemberLabelViewModel(
}
val currentState = internalUiState.value
memberLabelRepo.setLabel(
val result = memberLabelRepo.setLabel(
groupId = groupId,
label = MemberLabel(
emoji = currentState.labelEmoji.ifEmpty { null },
@@ -105,8 +107,24 @@ class MemberLabelViewModel(
)
)
val newSaveState: SaveState = when (result) {
is NetworkResult.Success -> SaveState.Success
is NetworkResult.NetworkError<*> -> SaveState.NetworkError
is NetworkResult.ApplicationError<*> -> {
if (result.throwable is GroupInsufficientRightsException) {
SaveState.InsufficientRights
} else {
throw result.throwable
}
}
is NetworkResult.StatusCodeError<*> -> throw result.exception
}
internalUiState.update {
it.copy(saveState = SaveState.Success)
it.copy(saveState = newSaveState)
}
}
}
@@ -142,5 +160,7 @@ data class MemberLabelUiState(
sealed interface SaveState {
data object InProgress : SaveState
data object Success : SaveState
data object NetworkError : SaveState
data object InsufficientRights : SaveState
}
}

View File

@@ -330,7 +330,7 @@
<!-- Accessibility label for the send button in media selection -->
<string name="CameraXFragment_send">Send</string>
<!-- CameraContacts -->
<!-- CameraContacts -->
<string name="CameraContacts_recent_contacts">Recent contacts</string>
<string name="CameraContacts_signal_contacts">Signal contacts</string>
<string name="CameraContacts_signal_groups">Signal groups</string>
@@ -4383,7 +4383,6 @@
<string name="PaymentsHomeFragment__update_now">Update now</string>
<!-- PaymentsSecuritySetupFragment -->
<!-- Toolbar title -->
<string name="PaymentsSecuritySetupFragment__security_setup">Security setup</string>
@@ -4957,7 +4956,7 @@
<string name="RegistrationActivity_error_connecting_to_service">Error connecting to service</string>
<string name="preferences_chats__backups">Backups</string>
<!-- Title text shown when Signal is locked and needs to be unlocked -->
<!-- Title text shown when Signal is locked and needs to be unlocked -->
<string name="prompt_passphrase_activity__unlock_signal">Unlock Signal</string>
<!-- Description text explaining how to unlock Signal -->
<string name="prompt_passphrase_activity__use_your_android_device">Use your Android device lock settings to unlock Signal.</string>
@@ -5986,8 +5985,6 @@
<string name="ConversationSettingsFragment__group_link">Group link</string>
<!-- Label for button that opens the group member label permissions screen. -->
<string name="ConversationSettingsFragment__group_member_label">Member Label</string>
<!-- Snackbar shown when non-admin taps the member label button -->
<string name="ConversationSettingsFragment__only_admins_can_add_member_labels">Only admins can add member labels in this group.</string>
<!-- Option in conversation settings to add a user as a contact -->
<string name="ConversationSettingsFragment__add_as_a_contact">Add as a contact</string>
<string name="ConversationSettingsFragment__unmute">Unmute</string>
@@ -7894,7 +7891,7 @@
<!-- Title of a megaphone shown at the bottom of the chat list to prompt the user to enable message backups -->
<string name="TurnOnSignalBackups__title">Turn on Signal Secure Backups</string>
<!-- Body of a megaphone shown at the bottom of the chat list to prompt the user to enable message backups -->
<!-- Body of a megaphone shown at the bottom of the chat list to prompt the user to enable message backups -->
<string name="TurnOnSignalBackups__body">Never lose a message when you get a new phone or reinstall Signal.</string>
<!-- Button of a megaphone shown at the bottom of the chat list to prompt the user to enable message backups that will take the user to the flow to enable backups -->
<string name="TurnOnSignalBackups__turn_on">Turn on</string>
@@ -9379,13 +9376,18 @@
<string name="GroupMemberLabel__preview_sample_message">Hello!</string>
<!-- Group member label save button label. -->
<string name="GroupMemberLabel__save">Save</string>
<!-- Description explaining the group member labels feature. -->
<string name="GroupMemberLabel__description">Add a member label to describe yourself or your role in this group. Labels are only visible within this group.</string>
<!-- Error message shown when the group member label fails to save due to a network error. -->
<string name="GroupMemberLabel__error_cant_save_no_network">Couldn\'t save label. Check your network and try again.</string>
<!-- Error message shown when trying to edit a member label without adequate permission. -->
<string name="GroupMemberLabel__error_no_edit_permission">Only admins can add member labels in this group.</string>
<!-- Accessibility label for the button to open the group member label emoji picker. -->
<string name="GroupMemberLabel__accessibility_select_emoji">Select emoji</string>
<!-- Accessibility label for the group member label close screen button. -->
<string name="GroupMemberLabel__accessibility_close_screen">Close screen</string>
<!-- Accessibility label for the group member label text field clear button. -->
<string name="GroupMemberLabel__accessibility_clear_label">Clear label</string>
<string name="GroupMemberLabel__description">Add a member label to describe yourself or your role in this group. Labels are only visible within this group.</string>
<!-- Title for the member labels education sheet. -->
<string name="MemberLabelsEducation__title">Member labels</string>

View File

@@ -19,8 +19,12 @@ import org.junit.Rule
import org.junit.Test
import org.thoughtcrime.securesms.conversation.colors.NameColor
import org.thoughtcrime.securesms.groups.GroupId
import org.thoughtcrime.securesms.groups.GroupInsufficientRightsException
import org.thoughtcrime.securesms.groups.memberlabel.MemberLabelUiState.SaveState
import org.thoughtcrime.securesms.recipients.RecipientId
import org.thoughtcrime.securesms.testing.CoroutineDispatcherRule
import org.whispersystems.signalservice.api.NetworkResult
import java.io.IOException
@OptIn(ExperimentalCoroutinesApi::class)
class MemberLabelViewModelTest {
@@ -180,6 +184,7 @@ class MemberLabelViewModelTest {
@Test
fun `save calls setLabel with truncated label when label exceeds max length`() = runTest(testDispatcher) {
coEvery { memberLabelRepo.getLabel(groupId, any<RecipientId>()) } returns null
coEvery { memberLabelRepo.setLabel(any(), any()) } returns NetworkResult.Success(Unit)
val viewModel = MemberLabelViewModel(memberLabelRepo, groupId, recipientId)
viewModel.onLabelTextChanged("A".repeat(30))
@@ -207,6 +212,7 @@ class MemberLabelViewModelTest {
@Test
fun `save calls setLabel when label change is valid`() = runTest(testDispatcher) {
coEvery { memberLabelRepo.getLabel(groupId, any<RecipientId>()) } returns MemberLabel(emoji = null, text = "Original")
coEvery { memberLabelRepo.setLabel(any(), any()) } returns NetworkResult.Success(Unit)
val viewModel = MemberLabelViewModel(memberLabelRepo, groupId, recipientId)
viewModel.onLabelTextChanged("New Label")
@@ -221,6 +227,7 @@ class MemberLabelViewModelTest {
@Test
fun `save calls setLabel with cleared values when clearLabel is called`() = runTest(testDispatcher) {
coEvery { memberLabelRepo.getLabel(groupId, any<RecipientId>()) } returns MemberLabel(emoji = "🎉", text = "Original")
coEvery { memberLabelRepo.setLabel(any(), any()) } returns NetworkResult.Success(Unit)
val viewModel = MemberLabelViewModel(memberLabelRepo, groupId, recipientId)
viewModel.clearLabel()
@@ -314,4 +321,40 @@ class MemberLabelViewModelTest {
assertTrue(viewModel.uiState.value.isSaveEnabled)
}
@Test
fun `save sets saveState to Success when setLabel returns NetworkResult Success`() = runTest(testDispatcher) {
coEvery { memberLabelRepo.getLabel(groupId, any<RecipientId>()) } returns MemberLabel(emoji = null, text = "Original")
coEvery { memberLabelRepo.setLabel(any(), any()) } returns NetworkResult.Success(Unit)
val viewModel = MemberLabelViewModel(memberLabelRepo, groupId, recipientId)
viewModel.onLabelTextChanged("New Label")
viewModel.save()
assertEquals(SaveState.Success, viewModel.uiState.value.saveState)
}
@Test
fun `save sets saveState to NetworkError when setLabel returns NetworkResult NetworkError`() = runTest(testDispatcher) {
coEvery { memberLabelRepo.getLabel(groupId, any<RecipientId>()) } returns MemberLabel(emoji = null, text = "Original")
coEvery { memberLabelRepo.setLabel(any(), any()) } returns NetworkResult.NetworkError(IOException("Network failure"))
val viewModel = MemberLabelViewModel(memberLabelRepo, groupId, recipientId)
viewModel.onLabelTextChanged("New Label")
viewModel.save()
assertEquals(SaveState.NetworkError, viewModel.uiState.value.saveState)
}
@Test
fun `save sets saveState to InsufficientRights when setLabel returns ApplicationError with GroupInsufficientRightsException`() = runTest(testDispatcher) {
coEvery { memberLabelRepo.getLabel(groupId, any<RecipientId>()) } returns MemberLabel(emoji = null, text = "Original")
coEvery { memberLabelRepo.setLabel(any(), any()) } returns NetworkResult.ApplicationError(GroupInsufficientRightsException(RuntimeException("Insufficient rights (test)")))
val viewModel = MemberLabelViewModel(memberLabelRepo, groupId, recipientId)
viewModel.onLabelTextChanged("New Label")
viewModel.save()
assertEquals(SaveState.InsufficientRights, viewModel.uiState.value.saveState)
}
}