Skip to content

Commit 19d1dbd

Browse files
authored
fix(chat): settle a denied send instead of offering a retry (#1760)
* fix(chat): store a denied send as its own status * fix(chat): settle a denied send instead of failing it The server answers Denied when it won't take a message from this user in this chat, so a retry gets the same answer. Text and photo sends now mark the row DENIED instead of FAILED, and the photo path drops its stored blob as it already does for EncryptionNotAllowed. * fix(chat): say a denied message can't be posted instead of offering a retry A DENIED row now reads "Not Delivered. You can't post in this chat" in the error colour, always shows, and isn't tappable. Photo bubbles stop offering Retry for any failure the send path marked non-retryable; they used to ignore that flag. * fix(chat): re-read a chat's rules after a denied send Denied carries no reason. If a speaker rule changed, the stored rules are stale and the composer stays up; re-reading the chat lets speakerBlock see the new rules and hide it. A removal from the group still isn't visible, because GetChat doesn't return the caller's membership. * fix(chat): stop reporting a denied send, typing notice or read pointer A denied send now settles as DENIED with its own message, and typing and read-pointer callers already ignore the result, so these refusals no longer need to reach Bugsnag. One user produced 6 of the 18 overnight events this way. * fix(chat): keep stored membership when re-reading after a denied send ChatEntityMapper.toEntity writes is_member = 1 by default. If the stream's MemberLeft landed before the re-read returned, the upsert put a removed user back in the feed as a member. The re-read now keeps whatever membership is stored.
1 parent 70f167b commit 19d1dbd

21 files changed

Lines changed: 221 additions & 16 deletions

File tree

‎apps/flipcash/core/src/main/res/values/strings.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -892,6 +892,7 @@
892892
<string name="label_chatReceipt_delivered">Delivered</string>
893893
<string name="label_chatReceipt_read">Read</string>
894894
<string name="label_chatReceipt_notSent">Not Delivered. Tap to retry</string>
895+
<string name="label_chatReceipt_denied">Not Delivered. You can\'t post in this chat</string>
895896
<string name="label_chatReceipt_yesterday">Yesterday</string>
896897
<string name="title_messageNotSent">Message Not Sent</string>
897898
<string name="description_messageNotSent">Your message could not be delivered. Would you like to try again?</string>

‎apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1171,6 +1171,7 @@ internal class ChatViewModel @AssistedInject constructor(
11711171
when (message.deliveryStatus) {
11721172
DeliveryStatus.SENDING -> ReceiptStatus.SENDING
11731173
DeliveryStatus.FAILED -> ReceiptStatus.FAILED
1174+
DeliveryStatus.DENIED -> ReceiptStatus.DENIED
11741175
DeliveryStatus.SENT -> ReceiptStatus.SENT
11751176
}
11761177
} else null

‎apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ReceiptLabel.kt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ internal fun ReceiptLabel(
6262
ReceiptStatus.SENT -> stringResource(R.string.label_chatReceipt_delivered)
6363
ReceiptStatus.READ -> stringResource(R.string.label_chatReceipt_read)
6464
ReceiptStatus.FAILED -> stringResource(R.string.label_chatReceipt_notSent)
65+
ReceiptStatus.DENIED -> stringResource(R.string.label_chatReceipt_denied)
6566
else -> return@AnimatedContent
6667
}
6768

@@ -82,7 +83,7 @@ internal fun ReceiptLabel(
8283
style = CodeTheme.typography.caption.copy(
8384
fontWeight = FontWeight.Bold,
8485
),
85-
color = if (animatedStatus == ReceiptStatus.FAILED) {
86+
color = if (animatedStatus == ReceiptStatus.FAILED || animatedStatus == ReceiptStatus.DENIED) {
8687
CodeTheme.colors.errorText
8788
} else {
8889
CodeTheme.colors.textSecondary

‎apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ReceiptRules.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ internal fun shouldShowReceiptLabel(
7171
// A split message's receipt goes under its last row only; the rows above share its status.
7272
if (!item.isLastRow) return false
7373
val status = effectiveReceiptStatus(item, otherReadPointer) ?: return false
74-
if (status == ReceiptStatus.FAILED) return true
74+
if (status == ReceiptStatus.FAILED || status == ReceiptStatus.DENIED) return true
7575
val confirmed = item.confirmedStatus(otherReadPointer, settlingKey) ?: return false
7676

7777
// Walking toward the newest: it keeps its line while nothing newer takes it -- no newer send

‎apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/components/ReceiptHandoffTest.kt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,13 @@ class ReceiptHandoffTest {
7575
assertEquals(listOf(true, true), shown(rows, settlingKey = send.messageKey))
7676
}
7777

78+
@Test
79+
fun `a denied send always shows its line`() {
80+
val send = mine(0, ReceiptStatus.DENIED, clientId = "c2")
81+
val rows = listOf(send, mine(1, ReceiptStatus.SENT))
82+
assertEquals(listOf(true, true), shown(rows, settlingKey = send.messageKey))
83+
}
84+
7885
@Test
7986
fun `a first send that is settling shows no line anywhere`() {
8087
val send = mine(1, ReceiptStatus.SENT, clientId = "c1")

‎apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/models/ChatListItem.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import com.flipcash.shared.chat.ui.UndecryptableHint
99
import com.getcode.opencode.model.core.ID
1010
import kotlin.time.Instant
1111

12-
enum class ReceiptStatus { SENDING, SENT, READ, FAILED }
12+
enum class ReceiptStatus { SENDING, SENT, READ, FAILED, DENIED }
1313

1414
sealed interface ChatListItem {
1515
val itemKey: Any

‎apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/media/MediaMessageBubble.kt‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,15 +151,18 @@ internal fun MediaMessageBubble(
151151
val progress by remember(item.pendingClientIdHex) {
152152
derivedStateOf { item.pendingClientIdHex?.let { progressOf()[it] } }
153153
}
154-
val failed = item.receiptStatus == ReceiptStatus.FAILED || progress is MediaSendProgress.Failed
154+
val denied = item.receiptStatus == ReceiptStatus.DENIED
155+
val failed = denied || item.receiptStatus == ReceiptStatus.FAILED || progress is MediaSendProgress.Failed
156+
// A denied or terminal failure still renders as failed, but a tap can't fix it.
157+
val retryable = !denied && (progress as? MediaSendProgress.Failed)?.retryable != false
155158

156159
var loadError by remember(model) { mutableStateOf<Throwable?>(null) }
157160
val unavailable = loadError?.let(::isPhotoUnavailable) == true
158161
val blurhashOnly = info?.blurhashOnly == true
159162

160163
val onClick: (() -> Unit)? = when {
161164
!interactive -> null
162-
failed -> ({ actionHandler(ChatAction.RetryMessage(item)) })
165+
failed -> if (retryable) ({ actionHandler(ChatAction.RetryMessage(item)) }) else null
163166
canOpenPhoto(blurhashOnly, failed = false, unavailable = unavailable) && localUri == null ->
164167
({ actionHandler(ChatAction.OpenPhoto(item)) })
165168
else -> null

‎apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/MediaSendDelegate.kt‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -333,9 +333,12 @@ class MediaSendDelegate internal constructor(
333333
}
334334
}
335335
.onFailure { cause ->
336-
// A chat that stopped taking ciphertext won't take this blob on a retry either.
337-
val terminal = cause is SendMessageError.EncryptionNotAllowed
338-
fail(chatId, clientMessageId, hex, retryable = !terminal, discard = record.takeIf { terminal })
336+
// A chat that stopped taking ciphertext won't take this blob on a retry either, and
337+
// a denied sender won't be let in by one.
338+
val denied = cause is SendMessageError.Denied
339+
val terminal = denied || cause is SendMessageError.EncryptionNotAllowed
340+
fail(chatId, clientMessageId, hex, retryable = !terminal, discard = record.takeIf { terminal }, denied = denied)
341+
if (denied) messaging.rereadAfterDenied(chatId)
339342
}
340343
}
341344

@@ -345,8 +348,13 @@ class MediaSendDelegate internal constructor(
345348
hex: String,
346349
retryable: Boolean,
347350
discard: PendingMediaRecord?,
351+
denied: Boolean = false,
348352
) {
349-
messageDataSource.failPending(chatId, clientMessageId)
353+
if (denied) {
354+
messageDataSource.denyPending(chatId, clientMessageId)
355+
} else {
356+
messageDataSource.failPending(chatId, clientMessageId)
357+
}
350358
stages.update { it + (hex to Stage.Failed(retryable)) }
351359
// A terminal failure has nothing a retry could resend, so the queue entry and the file go.
352360
if (discard != null) discard(discard)

‎apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/internal/delegates/MessagingDelegate.kt‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
package com.flipcash.shared.chat.internal.delegates
44

5+
import com.flipcash.services.models.SendMessageError
56
import androidx.core.app.NotificationManagerCompat
67
import androidx.paging.ExperimentalPagingApi
78
import androidx.paging.Pager
@@ -371,7 +372,7 @@ class MessagingDelegate @Inject constructor(
371372
/**
372373
* Seals [payload] if the chat encrypts, sends it, and settles the pending row. A failure to
373374
* seal fails the row like a failed request, so it can be retried; so does the server refusing
374-
* the ciphertext with `EncryptionNotAllowed`.
375+
* the ciphertext with `EncryptionNotAllowed`. A `Denied` settles the row as denied, with no retry.
375376
*/
376377
private suspend fun send(
377378
chatId: ChatId,
@@ -393,8 +394,14 @@ class MessagingDelegate @Inject constructor(
393394
metadataDataSource.updateLastMessageId(chatId, serverMessage.messageId)
394395
metadataDataSource.updateLastActivity(chatId, serverMessage.timestamp.toEpochMilliseconds())
395396
}
396-
.onFailure {
397-
messageDataSource.failPending(chatId, clientMessageId)
397+
.onFailure { cause ->
398+
// The server won't take this from this user in this chat; a retry gets the same answer.
399+
if (cause is SendMessageError.Denied) {
400+
messageDataSource.denyPending(chatId, clientMessageId)
401+
rereadAfterDenied(chatId)
402+
} else {
403+
messageDataSource.failPending(chatId, clientMessageId)
404+
}
398405
}
399406
}
400407

@@ -652,6 +659,21 @@ class MessagingDelegate @Inject constructor(
652659
senderResolver.clear()
653660
}
654661

662+
/**
663+
* The server refused a send without saying why. If a speaker rule changed, the stored rules are
664+
* stale and the composer is still up; re-reading the chat lets `speakerBlock` see the new rules.
665+
* A removal from the group is not visible here: GetChat does not return the caller's membership.
666+
*/
667+
internal suspend fun rereadAfterDenied(chatId: ChatId) {
668+
val metadata = chatController.getChat(chatId).getOrElse {
669+
trace(tag = TAG, message = "Couldn't re-read $chatId after a denied send", type = TraceType.Silent)
670+
return
671+
}
672+
// A removal may already have landed from the stream; don't write the caller back in.
673+
metadataDataSource.upsertKeepingMembership(metadata)
674+
memberDataSource.upsert(chatId, metadata.members)
675+
}
676+
655677
// endregion
656678

657679
private companion object {

‎apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/MessagingSendEncryptionTest.kt‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,62 @@ class MessagingSendEncryptionTest {
330330
coVerify { messages.failPending(chatId, clientMessageId) }
331331
}
332332

333+
@Test
334+
fun `Denied settles the send without a retry`() = runTest {
335+
storedChat(chat())
336+
answerSends()
337+
coEvery { controller.sendMessage(chatId, any(), any()) } returns
338+
Result.failure(SendMessageError.Denied())
339+
coEvery { chatController.getChat(chatId, any()) } returns Result.failure(GetChatError.Other())
340+
341+
val result = delegate.sendMessage(chatId, "hello", replyToMessageId = null)
342+
343+
assertIs<SendMessageError.Denied>(result.exceptionOrNull())
344+
coVerify { messages.denyPending(chatId, clientMessageId) }
345+
coVerify(exactly = 0) { messages.failPending(any(), any()) }
346+
}
347+
348+
@Test
349+
fun `Denied re-reads the chat so its rules can hide the composer`() = runTest {
350+
val stored = chat()
351+
storedChat(stored)
352+
answerSends()
353+
coEvery { controller.sendMessage(chatId, any(), any()) } returns Result.failure(SendMessageError.Denied())
354+
coEvery { chatController.getChat(chatId, any()) } returns Result.success(stored)
355+
356+
delegate.sendMessage(chatId, "hello", replyToMessageId = null)
357+
358+
coVerify { metadata.upsertKeepingMembership(stored) }
359+
coVerify(exactly = 0) { metadata.upsert(any<ChatMetadata>()) }
360+
coVerify { members.upsert(chatId, stored.members) }
361+
}
362+
363+
@Test
364+
fun `a failed re-read leaves the denial in place`() = runTest {
365+
storedChat(chat())
366+
answerSends()
367+
coEvery { controller.sendMessage(chatId, any(), any()) } returns Result.failure(SendMessageError.Denied())
368+
coEvery { chatController.getChat(chatId, any()) } returns Result.failure(GetChatError.Other())
369+
370+
val result = delegate.sendMessage(chatId, "hello", replyToMessageId = null)
371+
372+
assertIs<SendMessageError.Denied>(result.exceptionOrNull())
373+
coVerify { messages.denyPending(chatId, clientMessageId) }
374+
coVerify(exactly = 0) { metadata.upsertKeepingMembership(any()) }
375+
}
376+
377+
@Test
378+
fun `EncryptionNotAllowed does not re-read the chat`() = runTest {
379+
storedChat(chat())
380+
answerSends()
381+
coEvery { controller.sendMessage(chatId, any(), any()) } returns
382+
Result.failure(SendMessageError.EncryptionNotAllowed())
383+
384+
delegate.sendMessage(chatId, "hello", replyToMessageId = null)
385+
386+
coVerify(exactly = 0) { metadata.upsertKeepingMembership(any()) }
387+
}
388+
333389
@Test
334390
fun `a retry seals the stored plaintext again`() = runTest {
335391
storedChat(chat())

0 commit comments

Comments
 (0)