diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt index 50f63dab98..79765dfa8e 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt @@ -869,10 +869,10 @@ internal class ChatViewModel @Inject constructor( private fun initChatHandlers() { // Ahead of the transcript, so the first mapping already has the real windows rather than - // the defaults, which leave both edit and delete open. + // the fallbacks the default carries. userFlags.resolvedFlags .map { - MessagePolicy( + MessagePolicy.fromFlags( editWindow = it.messageEditWindow.effectiveValue, deleteWindow = it.messageDeleteWindow.effectiveValue, ) diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatMessageActionReducerTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatMessageActionReducerTest.kt index 36cb1294bb..790879ae2b 100644 --- a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatMessageActionReducerTest.kt +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatMessageActionReducerTest.kt @@ -27,6 +27,15 @@ class ChatMessageActionReducerTest { private val sentAt = Instant.fromEpochSeconds(1_000) + /** + * Both windows open. `MessagePolicy.Default` carries the fallback windows, and these bubbles + * are timestamped in 1970, so the default would strip Edit and Delete from every selection + * before the assertion could tell whether the reducer stored the right bubble. The window rules + * themselves are covered in `MessageCapabilityTest`; the two cases below that do exercise a + * window state it directly. + */ + private val unbounded = MessagePolicy(editWindow = null, deleteWindow = null) + private fun bubble( messageId: Long, text: String = "hello", @@ -54,7 +63,7 @@ class ChatMessageActionReducerTest { val target = bubble(1) val state = reduce( - ChatViewModel.State(), + ChatViewModel.State(messagePolicy = unbounded), ChatViewModel.Event.ToggleMessageSelection(target), ) @@ -68,7 +77,7 @@ class ChatMessageActionReducerTest { // window that has since closed. Offering Edit anyway costs a round-trip the server answers // CANNOT_EDIT. sentAt is decades old, so any finite window here has closed. val state = reduce( - ChatViewModel.State(messagePolicy = MessagePolicy(editWindow = 5.minutes)), + ChatViewModel.State(messagePolicy = MessagePolicy(editWindow = 5.minutes, deleteWindow = null)), ChatViewModel.Event.ToggleMessageSelection(bubble(1)), ) @@ -82,7 +91,7 @@ class ChatMessageActionReducerTest { fun `selecting a bubble past its delete window drops Delete`() { // Separate from the case above so a window wired to the wrong capability cannot pass both. val state = reduce( - ChatViewModel.State(messagePolicy = MessagePolicy(deleteWindow = 5.minutes)), + ChatViewModel.State(messagePolicy = MessagePolicy(editWindow = null, deleteWindow = 5.minutes)), ChatViewModel.Event.ToggleMessageSelection(bubble(1)), ) @@ -96,7 +105,7 @@ class ChatMessageActionReducerTest { fun `long-pressing the selected bubble again clears the bar`() { val target = bubble(1) val selected = reduce( - ChatViewModel.State(), + ChatViewModel.State(messagePolicy = unbounded), ChatViewModel.Event.ToggleMessageSelection(target), ) @@ -111,7 +120,7 @@ class ChatMessageActionReducerTest { val first = bubble(1) val second = bubble(2, text = "goodbye") val selected = reduce( - ChatViewModel.State(), + ChatViewModel.State(messagePolicy = unbounded), ChatViewModel.Event.ToggleMessageSelection(first), ) @@ -123,7 +132,7 @@ class ChatMessageActionReducerTest { @Test fun `copying clears the bar`() { val selected = reduce( - ChatViewModel.State(), + ChatViewModel.State(messagePolicy = unbounded), ChatViewModel.Event.ToggleMessageSelection(bubble(1)), ) @@ -138,7 +147,7 @@ class ChatMessageActionReducerTest { // focus goes because the sheet is modal — a sharp bubble behind it reads as still live. val target = bubble(1) val selected = reduce( - ChatViewModel.State(), + ChatViewModel.State(messagePolicy = unbounded), ChatViewModel.Event.ToggleMessageSelection(target), ) @@ -153,7 +162,7 @@ class ChatMessageActionReducerTest { // Confirmed or cancelled, the sheet's close is ClearMessageSelection, which the handler // drives. Leaving confirmingDelete set would hold the whole transcript behind the backdrop. val confirming = reduce( - reduce(ChatViewModel.State(), ChatViewModel.Event.ToggleMessageSelection(bubble(1))), + reduce(ChatViewModel.State(messagePolicy = unbounded), ChatViewModel.Event.ToggleMessageSelection(bubble(1))), ChatViewModel.Event.DeleteMessage(1), ) @@ -167,7 +176,7 @@ class ChatMessageActionReducerTest { fun `starting an edit takes over the composer and stashes the draft`() { val target = bubble(1) val selected = reduce( - ChatViewModel.State(chatInputState = TextFieldState("half-written")), + ChatViewModel.State(messagePolicy = unbounded, chatInputState = TextFieldState("half-written")), ChatViewModel.Event.ToggleMessageSelection(target), ) @@ -186,7 +195,7 @@ class ChatMessageActionReducerTest { @Test fun `editing a second message keeps the original draft rather than the first edit's text`() { val first = reduce( - ChatViewModel.State(chatInputState = TextFieldState("half-written")), + ChatViewModel.State(messagePolicy = unbounded, chatInputState = TextFieldState("half-written")), ChatViewModel.Event.EditMessage(1, "hello"), ) // The composer now holds the first message's body, which is not the user's draft. @@ -204,7 +213,7 @@ class ChatMessageActionReducerTest { fun `ending an edit releases the composer`() { // Confirm, cancel and back all land here; restoring the stashed draft is the handler's job. val editing = reduce( - ChatViewModel.State(chatInputState = TextFieldState("half-written")), + ChatViewModel.State(messagePolicy = unbounded, chatInputState = TextFieldState("half-written")), ChatViewModel.Event.EditMessage(1, "hello"), ) @@ -216,7 +225,7 @@ class ChatMessageActionReducerTest { @Test fun `submitting and cancelling leave the edit in place for the handler to read`() { val editing = reduce( - ChatViewModel.State(), + ChatViewModel.State(messagePolicy = unbounded), ChatViewModel.Event.EditMessage(1, "hello"), ) @@ -236,7 +245,7 @@ class ChatMessageActionReducerTest { fun `replying opens the strip and clears the selection`() { val target = bubble(1) val selected = reduce( - ChatViewModel.State(), + ChatViewModel.State(messagePolicy = unbounded), ChatViewModel.Event.ToggleMessageSelection(target), ) @@ -254,7 +263,7 @@ class ChatMessageActionReducerTest { @Test fun `replying leaves the draft in the composer`() { val state = reduce( - ChatViewModel.State(chatInputState = TextFieldState("half-written")), + ChatViewModel.State(messagePolicy = unbounded, chatInputState = TextFieldState("half-written")), ChatViewModel.Event.ReplyToMessage(quote()), ) @@ -264,7 +273,7 @@ class ChatMessageActionReducerTest { @Test fun `starting an edit takes the reply strip down`() { val replying = reduce( - ChatViewModel.State(), + ChatViewModel.State(messagePolicy = unbounded), ChatViewModel.Event.ReplyToMessage(quote()), ) @@ -277,7 +286,7 @@ class ChatMessageActionReducerTest { @Test fun `replying takes an edit down`() { val editing = reduce( - ChatViewModel.State(), + ChatViewModel.State(messagePolicy = unbounded), ChatViewModel.Event.EditMessage(1, "hello"), ) @@ -290,7 +299,7 @@ class ChatMessageActionReducerTest { @Test fun `cancelling the reply keeps the draft`() { val replying = reduce( - ChatViewModel.State(chatInputState = TextFieldState("half-written")), + ChatViewModel.State(messagePolicy = unbounded, chatInputState = TextFieldState("half-written")), ChatViewModel.Event.ReplyToMessage(quote()), ) @@ -308,7 +317,7 @@ class ChatMessageActionReducerTest { @Test fun `sending leaves the reply in place for the handler to read`() { val replying = reduce( - ChatViewModel.State(), + ChatViewModel.State(messagePolicy = unbounded), ChatViewModel.Event.ReplyToMessage(quote()), ) @@ -323,14 +332,14 @@ class ChatMessageActionReducerTest { */ @Test fun `a jump request alone sets no target`() { - val state = reduce(ChatViewModel.State(), ChatViewModel.Event.JumpToMessage(7)) + val state = reduce(ChatViewModel.State(messagePolicy = unbounded), ChatViewModel.Event.JumpToMessage(7)) assertNull(state.jumpTarget) } @Test fun `a resolved jump carries the target and its bound`() { - val state = reduce(ChatViewModel.State(), ChatViewModel.Event.JumpResolved(7, 240)) + val state = reduce(ChatViewModel.State(messagePolicy = unbounded), ChatViewModel.Event.JumpResolved(7, 240)) assertEquals(7L, state.jumpTarget) assertEquals(240, state.jumpBudget) @@ -338,7 +347,7 @@ class ChatMessageActionReducerTest { @Test fun `consuming a jump clears both`() { - val jumping = reduce(ChatViewModel.State(), ChatViewModel.Event.JumpResolved(7, 240)) + val jumping = reduce(ChatViewModel.State(messagePolicy = unbounded), ChatViewModel.Event.JumpResolved(7, 240)) val state = reduce(jumping, ChatViewModel.Event.JumpConsumed) diff --git a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/MessageCapability.kt b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/MessageCapability.kt index 8265cd96f8..34d91cd236 100644 --- a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/MessageCapability.kt +++ b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/MessageCapability.kt @@ -4,6 +4,8 @@ import com.flipcash.services.models.chat.ChatMessage import com.flipcash.services.models.chat.MessageContent import kotlin.time.Clock import kotlin.time.Duration +import kotlin.time.Duration.Companion.hours +import kotlin.time.Duration.Companion.minutes import kotlin.time.Instant /** @@ -30,19 +32,59 @@ enum class MessageCapability { * Client-side limits on what may be done to a message. * * Both windows come from `UserFlags` (`message_edit_window`, `message_delete_window`), which sends - * them with explicit presence: an unset field is no limit rather than a zero-length one. The - * defaults are therefore both `null`, which leaves `CANNOT_EDIT` / `CANNOT_DELETE` as the - * authority for a build that has not seen the flags yet. + * them with explicit presence: an unset field is distinguishable from a zero-length one. Where the + * server sends nothing, [fromFlags] substitutes [FallbackEditWindow] / [FallbackDeleteWindow] + * rather than leaving the action open forever, so a message old enough can lose Edit or Delete + * even where the server would have taken the request. That is the accepted cost: an affordance the + * server answers `CANNOT_EDIT` / `CANNOT_DELETE` is the worse failure. + * + * Neither window has a default here. Absence is a real input — it decides whether the fallback + * applies — so it is worth stating at the call site rather than inheriting. * * @param editWindow how long after sending a message stays editable, or `null` for no limit. * @param deleteWindow how long after sending a message stays deletable, or `null` for no limit. */ data class MessagePolicy( - val editWindow: Duration? = null, - val deleteWindow: Duration? = null, + val editWindow: Duration?, + val deleteWindow: Duration?, ) { companion object { - val Default = MessagePolicy() + /** + * The window applied when the server sends no edit window. + * + * Maintained in parallel with iOS `MessagePolicy.fallbackEditWindow` + * (`FlipcashCore/Sources/FlipcashCore/Models/Conversation/MessagePolicy.swift`). The two + * must move together or the clients offer different rows for the same message; nothing + * enforces it, so changing one means changing the other in the same release. + * + * The value is a product choice, not a figure the contract supplies: `message_edit_window` + * documents what it means but never what an absent field implies. Replace it the moment the + * server does specify one. + */ + val FallbackEditWindow = 15.minutes + + /** + * The window applied when the server sends no delete window. Same parallel-maintenance duty + * and same provenance as [FallbackEditWindow]; iOS holds it as + * `MessagePolicy.fallbackDeleteWindow`. + */ + val FallbackDeleteWindow = 48.hours + + /** + * Builds the policy in force from the windows the server sent, substituting the fallbacks + * for anything it left unset. + * + * Both arguments are nullable because every upstream state collapses to the same one: + * flags not yet fetched, a fetch that failed, and flags whose window fields are unset all + * arrive as `null` and all get the fallback. There is no second path to keep in step. + */ + fun fromFlags(editWindow: Duration?, deleteWindow: Duration?) = MessagePolicy( + editWindow = editWindow ?: FallbackEditWindow, + deleteWindow = deleteWindow ?: FallbackDeleteWindow, + ) + + /** The policy in force before any flags have been read: the fallback windows. */ + val Default = fromFlags(editWindow = null, deleteWindow = null) } } diff --git a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/MessageCapabilityTest.kt b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/MessageCapabilityTest.kt index 1cc790b0e4..f8e80af6d6 100644 --- a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/MessageCapabilityTest.kt +++ b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/MessageCapabilityTest.kt @@ -9,6 +9,7 @@ import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner import kotlin.test.assertEquals import kotlin.time.Duration.Companion.days +import kotlin.time.Duration.Companion.hours import kotlin.time.Duration.Companion.minutes import kotlin.time.Instant @@ -53,6 +54,8 @@ class MessageCapabilityTest { isFromSelf, ) + // Resolved at the instant it was sent, so the default policy's fallback windows are both open + // and this stays a statement about content rather than about age. @Test fun `own text message is copyable, editable and deletable`() { assertEquals( @@ -62,7 +65,7 @@ class MessageCapabilityTest { MessageCapability.Edit, MessageCapability.Delete, ), - resolveCapabilities(text()), + resolveCapabilities(text(), now = sentAt), ) } @@ -116,13 +119,13 @@ class MessageCapabilityTest { MessageCapability.Edit, MessageCapability.Delete, ), - resolveCapabilities(reply), + resolveCapabilities(reply, now = sentAt), ) } @Test fun `an edit window drops Edit once it lapses and leaves Delete alone`() { - val policy = MessagePolicy(editWindow = 15.minutes) + val policy = MessagePolicy(editWindow = 15.minutes, deleteWindow = null) assertEquals( setOf( @@ -142,7 +145,7 @@ class MessageCapabilityTest { @Test fun `a delete window drops Delete once it lapses and leaves Edit alone`() { - val policy = MessagePolicy(deleteWindow = 60.minutes) + val policy = MessagePolicy(editWindow = null, deleteWindow = 60.minutes) assertEquals( setOf( @@ -177,6 +180,8 @@ class MessageCapabilityTest { @Test fun `an unset window leaves its capability open`() { + val unbounded = MessagePolicy(editWindow = null, deleteWindow = null) + assertEquals( setOf( MessageCapability.Copy, @@ -184,10 +189,63 @@ class MessageCapabilityTest { MessageCapability.Edit, MessageCapability.Delete, ), - resolveCapabilities(text(), MessagePolicy.Default, now = sentAt + 365.days), + resolveCapabilities(text(), unbounded, now = sentAt + 365.days), ) } + @Test + fun `windows the server did not send fall back rather than staying open`() { + val policy = MessagePolicy.fromFlags(editWindow = null, deleteWindow = null) + + assertEquals(MessagePolicy.FallbackEditWindow, policy.editWindow) + assertEquals(MessagePolicy.FallbackDeleteWindow, policy.deleteWindow) + assertEquals(policy, MessagePolicy.Default) + + assertEquals( + setOf(MessageCapability.Copy, MessageCapability.Reply, MessageCapability.Delete), + resolveCapabilities(text(), policy, now = sentAt + 30.minutes), + ) + assertEquals( + setOf(MessageCapability.Copy, MessageCapability.Reply), + resolveCapabilities(text(), policy, now = sentAt + 365.days), + ) + } + + /** + * The fallback covers only what the server left unset, so a window it did send has to survive + * the substitution — including one longer than the fallback, which is where a `?:` on the wrong + * side of the expression would show up. + */ + @Test + fun `windows the server did send are used as sent`() { + val policy = MessagePolicy.fromFlags(editWindow = 90.minutes, deleteWindow = null) + + assertEquals(90.minutes, policy.editWindow) + assertEquals(MessagePolicy.FallbackDeleteWindow, policy.deleteWindow) + + assertEquals( + setOf( + MessageCapability.Copy, + MessageCapability.Reply, + MessageCapability.Edit, + MessageCapability.Delete, + ), + resolveCapabilities(text(), policy, now = sentAt + 60.minutes), + ) + } + + /** + * Both numbers are maintained by hand against iOS `MessagePolicy.fallbackEditWindow` / + * `fallbackDeleteWindow`. Nothing checks the two repos against each other, so this pins the + * Android side: a change here fails until someone states the new value, which is the prompt to + * go and change iOS too. + */ + @Test + fun `the fallback windows are the values iOS carries`() { + assertEquals(15.minutes, MessagePolicy.FallbackEditWindow) + assertEquals(48.hours, MessagePolicy.FallbackDeleteWindow) + } + /** * The transcript resolves once, when it is mapped; the menu re-applies the windows when it * opens. Both go through the same rule, so a set narrowed after the fact matches what the