diff --git a/apps/ios/Shared/Views/Chat/ChatInfoView.swift b/apps/ios/Shared/Views/Chat/ChatInfoView.swift index c17d8e23a8..29c87ff7e0 100644 --- a/apps/ios/Shared/Views/Chat/ChatInfoView.swift +++ b/apps/ios/Shared/Views/Chat/ChatInfoView.swift @@ -577,7 +577,7 @@ struct ChatInfoView: View { private func clearChatAlert() -> Alert { Alert( title: Text("Clear conversation?"), - message: Text("All messages will be deleted - this cannot be undone! The messages will be deleted ONLY for you."), + message: Text(chat.chatInfo.displayName + "\n\n") + Text("All messages will be deleted - this cannot be undone! The messages will be deleted ONLY for you."), primaryButton: .destructive(Text("Clear")) { Task { await clearChat(chat) @@ -1185,6 +1185,7 @@ private func deleteContactOrConversationDialog( showActionSheet(SomeActionSheet( actionSheet: ActionSheet( title: Text("Delete contact?"), + message: Text(contact.displayName), buttons: [ .destructive(Text("Only delete conversation")) { deleteContactMaybeErrorAlert(chat, contact, chatDeleteMode: .messages, dismissToChatList, showAlert) @@ -1331,6 +1332,7 @@ private func deleteContactWithoutConversation( showActionSheet(SomeActionSheet( actionSheet: ActionSheet( title: Text("Confirm contact deletion?"), + message: Text(contact.displayName), buttons: [ .destructive(Text("Delete and notify contact")) { deleteContactMaybeErrorAlert(chat, contact, chatDeleteMode: .full(notify: true), dismissToChatList, showAlert) @@ -1355,6 +1357,7 @@ private func deleteNotReadyContact( showActionSheet(SomeActionSheet( actionSheet: ActionSheet( title: Text("Confirm contact deletion?"), + message: Text(contact.displayName), buttons: [ .destructive(Text("Confirm")) { deleteContactMaybeErrorAlert(chat, contact, chatDeleteMode: .full(notify: false), dismissToChatList, showAlert) diff --git a/apps/ios/Shared/Views/Chat/Group/GroupChatInfoView.swift b/apps/ios/Shared/Views/Chat/Group/GroupChatInfoView.swift index 0a448a2772..1353f590fa 100644 --- a/apps/ios/Shared/Views/Chat/Group/GroupChatInfoView.swift +++ b/apps/ios/Shared/Views/Chat/Group/GroupChatInfoView.swift @@ -845,7 +845,7 @@ struct GroupChatInfoView: View { let label: LocalizedStringKey = groupInfo.useRelays ? "Delete channel?" : groupInfo.businessChat == nil ? "Delete group?" : "Delete chat?" return Alert( title: Text(label), - message: deleteGroupAlertMessage(groupInfo), + message: Text(chat.chatInfo.displayName + "\n\n") + deleteGroupAlertMessage(groupInfo), primaryButton: .destructive(Text("Delete")) { Task { do { @@ -867,7 +867,7 @@ struct GroupChatInfoView: View { private func clearChatAlert() -> Alert { Alert( title: Text("Clear conversation?"), - message: Text("All messages will be deleted - this cannot be undone! The messages will be deleted ONLY for you."), + message: Text(chat.chatInfo.displayName + "\n\n") + Text("All messages will be deleted - this cannot be undone! The messages will be deleted ONLY for you."), primaryButton: .destructive(Text("Clear")) { Task { await clearChat(chat) @@ -889,7 +889,7 @@ struct GroupChatInfoView: View { ) return Alert( title: Text(titleLabel), - message: Text(messageLabel), + message: Text(chat.chatInfo.displayName + "\n\n") + Text(messageLabel), primaryButton: .destructive(Text("Leave")) { Task { await leaveGroup(chat.chatInfo.apiId) diff --git a/apps/ios/Shared/Views/ChatList/ChatListNavLink.swift b/apps/ios/Shared/Views/ChatList/ChatListNavLink.swift index b4590fc124..76734dcb42 100644 --- a/apps/ios/Shared/Views/ChatList/ChatListNavLink.swift +++ b/apps/ios/Shared/Views/ChatList/ChatListNavLink.swift @@ -568,7 +568,7 @@ struct ChatListNavLink: View { let label: LocalizedStringKey = groupInfo.useRelays ? "Delete channel?" : groupInfo.businessChat == nil ? "Delete group?" : "Delete chat?" return Alert( title: Text(label), - message: deleteGroupAlertMessage(groupInfo), + message: Text(chat.chatInfo.displayName + "\n\n") + deleteGroupAlertMessage(groupInfo), primaryButton: .destructive(Text("Delete")) { Task { await deleteChat(chat) } }, @@ -600,7 +600,7 @@ struct ChatListNavLink: View { private func clearChatAlert() -> Alert { Alert( title: Text("Clear conversation?"), - message: Text("All messages will be deleted - this cannot be undone! The messages will be deleted ONLY for you."), + message: Text(chat.chatInfo.displayName + "\n\n") + Text("All messages will be deleted - this cannot be undone! The messages will be deleted ONLY for you."), primaryButton: .destructive(Text("Clear")) { Task { await clearChat(chat) } }, @@ -630,7 +630,7 @@ struct ChatListNavLink: View { ) return Alert( title: Text(titleLabel), - message: Text(messageLabel), + message: Text(chat.chatInfo.displayName + "\n\n") + Text(messageLabel), primaryButton: .destructive(Text("Leave")) { Task { await leaveGroup(groupInfo.groupId) } }, @@ -701,10 +701,10 @@ func rejectContactRequestAlert(_ contactRequestId: Int64) -> Alert { func deleteContactConnectionAlert(_ contactConnection: PendingContactConnection, showError: @escaping (ErrorAlert) -> Void, success: @escaping () -> Void = {}) -> Alert { Alert( title: Text("Delete pending connection?"), - message: - contactConnection.initiated - ? Text("The contact you shared this link with will NOT be able to connect!") - : Text("The connection you accepted will be cancelled!"), + message: Text(contactConnection.displayName + "\n\n") + + (contactConnection.initiated + ? Text("The contact you shared this link with will NOT be able to connect!") + : Text("The connection you accepted will be cancelled!")), primaryButton: .destructive(Text("Delete")) { Task { do { diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ChatInfoView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ChatInfoView.kt index dce1b6ea33..13be8b1d71 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ChatInfoView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ChatInfoView.kt @@ -248,6 +248,8 @@ fun deleteContactDialog(chat: Chat, chatModel: ChatModel, close: (() -> Unit)? = private fun deleteContactOrConversationDialog(chat: Chat, contact: Contact, chatModel: ChatModel, close: (() -> Unit)?) { AlertManager.shared.showAlertDialogButtonsColumn( title = generalGetString(MR.strings.delete_contact_question), + text = contact.displayName, + parseHtml = false, buttons = { Column { // Only delete conversation @@ -306,7 +308,8 @@ private fun deleteActiveContactDialog(chat: Chat, contact: Contact, chatModel: C AlertManager.shared.showAlertDialogButtonsColumn( title = generalGetString(MR.strings.delete_contact_question), - text = generalGetString(MR.strings.delete_contact_cannot_undo_warning), + text = "${contact.displayName}\n\n${generalGetString(MR.strings.delete_contact_cannot_undo_warning)}", + parseHtml = false, buttons = { Column { // Keep conversation toggle @@ -361,7 +364,8 @@ private fun deleteActiveContactDialog(chat: Chat, contact: Contact, chatModel: C private fun deleteContactWithoutConversation(chat: Chat, chatModel: ChatModel, close: (() -> Unit)?) { AlertManager.shared.showAlertDialogButtonsColumn( title = generalGetString(MR.strings.confirm_delete_contact_question), - text = generalGetString(MR.strings.delete_contact_cannot_undo_warning), + text = "${chat.chatInfo.displayName}\n\n${generalGetString(MR.strings.delete_contact_cannot_undo_warning)}", + parseHtml = false, buttons = { Column { // Delete and notify contact @@ -417,7 +421,8 @@ private fun deleteContactWithoutConversation(chat: Chat, chatModel: ChatModel, c private fun deleteNotReadyContact(chat: Chat, chatModel: ChatModel, close: (() -> Unit)?) { AlertManager.shared.showAlertDialogButtonsColumn( title = generalGetString(MR.strings.confirm_delete_contact_question), - text = generalGetString(MR.strings.delete_contact_cannot_undo_warning), + text = "${chat.chatInfo.displayName}\n\n${generalGetString(MR.strings.delete_contact_cannot_undo_warning)}", + parseHtml = false, buttons = { // Confirm SectionItemView({ @@ -492,7 +497,8 @@ fun deleteContact(chat: Chat, chatModel: ChatModel, close: (() -> Unit)?, chatDe fun clearChatDialog(chat: Chat, close: (() -> Unit)? = null) { AlertManager.shared.showAlertDialog( title = generalGetString(MR.strings.clear_chat_question), - text = generalGetString(MR.strings.clear_chat_warning), + text = "${chat.chatInfo.displayName}\n\n${generalGetString(MR.strings.clear_chat_warning)}", + parseHtml = false, confirmText = generalGetString(MR.strings.clear_verb), onConfirm = { controller.clearChat(chat, close) }, destructive = true, diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/group/GroupChatInfoView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/group/GroupChatInfoView.kt index 3e9b5f6f48..c88e3b2bff 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/group/GroupChatInfoView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/group/GroupChatInfoView.kt @@ -199,7 +199,8 @@ fun deleteGroupDialog(chat: Chat, groupInfo: GroupInfo, chatModel: ChatModel, cl } AlertManager.shared.showAlertDialog( title = generalGetString(titleId), - text = generalGetString(messageId), + text = "${groupInfo.displayName}\n\n${generalGetString(messageId)}", + parseHtml = false, confirmText = generalGetString(MR.strings.delete_verb), onConfirm = { withBGApi { @@ -233,7 +234,8 @@ fun leaveGroupDialog(rhId: Long?, groupInfo: GroupInfo, chatModel: ChatModel, cl MR.strings.you_will_stop_receiving_messages_from_this_chat_chat_history_will_be_preserved AlertManager.shared.showAlertDialog( title = generalGetString(titleId), - text = generalGetString(messageId), + text = "${groupInfo.displayName}\n\n${generalGetString(messageId)}", + parseHtml = false, confirmText = generalGetString(MR.strings.leave_group_button), onConfirm = { withLongRunningApi(60_000) { diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.kt index d3533bbd02..7dfb52063e 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.kt @@ -772,10 +772,11 @@ fun rejectContactRequest(rhId: Long?, contactRequestId: Long, chatModel: ChatMod fun deleteContactConnectionAlert(rhId: Long?, connection: PendingContactConnection, chatModel: ChatModel, onSuccess: () -> Unit) { AlertManager.shared.showAlertDialog( title = generalGetString(MR.strings.delete_pending_connection__question), - text = generalGetString( + text = "${connection.displayName}\n\n" + generalGetString( if (connection.initiated) MR.strings.contact_you_shared_link_with_wont_be_able_to_connect else MR.strings.connection_you_accepted_will_be_cancelled ), + parseHtml = false, confirmText = generalGetString(MR.strings.delete_verb), onConfirm = { withBGApi { diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/helpers/AlertManager.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/helpers/AlertManager.kt index 3d670d1c43..1774cf4394 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/helpers/AlertManager.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/helpers/AlertManager.kt @@ -75,6 +75,8 @@ class AlertManager { onDismissRequest: (() -> Unit)? = null, hostDevice: Pair? = null, belowTextContent: @Composable (() -> Unit) = {}, + // When false, [text] is rendered as literal text — use for user-controlled content. + parseHtml: Boolean = true, buttons: @Composable () -> Unit, ) { showAlert { @@ -82,8 +84,14 @@ class AlertManager { onDismissRequest = { onDismissRequest?.invoke(); if (dismissible) hideAlert() }, title = alertTitle(title), buttons = { - AlertContent(text, hostDevice, extraPadding = true, textAlign = textAlign, belowTextContent = belowTextContent) { - buttons() + if (parseHtml) { + AlertContent(text, hostDevice, extraPadding = true, textAlign = textAlign, belowTextContent = belowTextContent) { + buttons() + } + } else { + AlertContent(text?.let { AnnotatedString(it) }, hostDevice, extraPadding = true) { + buttons() + } } }, shape = RoundedCornerShape(corner = CornerSize(25.dp)) @@ -122,13 +130,15 @@ class AlertManager { onDismissRequest: (() -> Unit)? = null, destructive: Boolean = false, hostDevice: Pair? = null, + // When false, [text] is rendered as literal text — use for user-controlled content. + parseHtml: Boolean = true, ) { showAlert { AlertDialog( onDismissRequest = { onDismissRequest?.invoke(); hideAlert() }, title = alertTitle(title), buttons = { - AlertContent(text, hostDevice, true) { + val buttonRow: @Composable () -> Unit = { Row( Modifier.fillMaxWidth().padding(horizontal = DEFAULT_PADDING), horizontalArrangement = Arrangement.SpaceBetween @@ -149,6 +159,11 @@ class AlertManager { }, Modifier.focusRequester(focusRequester)) { Text(confirmText, color = if (destructive) MaterialTheme.colors.error else Color.Unspecified) } } } + if (parseHtml) { + AlertContent(text, hostDevice, true, content = buttonRow) + } else { + AlertContent(text?.let { AnnotatedString(it) }, hostDevice, true, content = buttonRow) + } }, shape = RoundedCornerShape(corner = CornerSize(25.dp)) ) diff --git a/plans/delete-leave-dialog-with-profile-impl.md b/plans/delete-leave-dialog-with-profile-impl.md new file mode 100644 index 0000000000..860d555d36 --- /dev/null +++ b/plans/delete-leave-dialog-with-profile-impl.md @@ -0,0 +1,323 @@ +# Implementation plan — chat name on its own line in delete/leave/clear dialogs + +Follows the product spec in +[`delete-leave-dialog-with-profile.md`](./delete-leave-dialog-with-profile.md). + +Pure code change — zero string additions, zero new helpers, zero +signature changes. Each call site edits one argument: the `text =` / +`message:` value gains `"${displayName}\n\n"` prepended to the +existing localized warning (or, where there is no current body, the +chat name becomes the new body). + +One commit per platform. + +## Commit 1 — Kotlin + +**Files touched:** +- `apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/helpers/AlertManager.kt` + — adds `parseHtml: Boolean = true` to `showAlertDialog` and + `showAlertDialogButtonsColumn`. When `false`, the body text is wrapped + as `AnnotatedString` and routed through the existing AnnotatedString + `AlertContent` overload, which does NOT call + `escapedHtmlToAnnotatedString`. Default stays `true` so existing + callers are unaffected. +- `apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/group/GroupChatInfoView.kt` +- `apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ChatInfoView.kt` +- `apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.kt` + — adds the previously-missed `deleteContactConnectionAlert` + dispatcher to the coverage (pending contact connections). + +Every Kotlin call site that prepends the chat name sets +`parseHtml = false`, so `displayName` is never HTML-interpreted. + +### 1.1 — `deleteGroupDialog` (`GroupChatInfoView.kt:182`) + +```diff + fun deleteGroupDialog(chat: Chat, groupInfo: GroupInfo, chatModel: ChatModel, close: (() -> Unit)? = null) { + val chatInfo = chat.chatInfo + val titleId = /* unchanged */ + val messageId = /* unchanged */ + AlertManager.shared.showAlertDialog( + title = generalGetString(titleId), +- text = generalGetString(messageId), ++ text = "${groupInfo.displayName}\n\n${generalGetString(messageId)}", + confirmText = generalGetString(MR.strings.delete_verb), + onConfirm = { /* unchanged */ }, + destructive = true, + ) + } +``` + +### 1.2 — `leaveGroupDialog` (`GroupChatInfoView.kt:222`) + +```diff + fun leaveGroupDialog(rhId: Long?, groupInfo: GroupInfo, chatModel: ChatModel, close: (() -> Unit)? = null) { + val titleId = /* unchanged */ + val messageId = /* unchanged */ + AlertManager.shared.showAlertDialog( + title = generalGetString(titleId), +- text = generalGetString(messageId), ++ text = "${groupInfo.displayName}\n\n${generalGetString(messageId)}", + confirmText = generalGetString(MR.strings.leave_group_button), + onConfirm = { /* unchanged */ }, + destructive = true, + ) + } +``` + +Signature unchanged. No caller updates. `groupInfo.displayName` is +already available on the existing parameter (`ChatModel.kt:2142`). + +### 1.3 — `clearChatDialog` (`ChatInfoView.kt:492`) + +```diff + fun clearChatDialog(chat: Chat, close: (() -> Unit)? = null) { + AlertManager.shared.showAlertDialog( + title = generalGetString(MR.strings.clear_chat_question), +- text = generalGetString(MR.strings.clear_chat_warning), ++ text = "${chat.chatInfo.displayName}\n\n${generalGetString(MR.strings.clear_chat_warning)}", + confirmText = generalGetString(MR.strings.clear_verb), + onConfirm = { controller.clearChat(chat, close) }, + destructive = true, + ) + } +``` + +### 1.4 — Contact-delete dispatchers (`ChatInfoView.kt`) + +Four functions. `deleteContactOrConversationDialog` (line 248) has +no existing `text =`, so the chat name becomes the new body. The +other three already have a `text =`, so the name is prepended. + +All four already have `contact: Contact` as a parameter, so +`contact.displayName` is used directly (same value as +`chat.chatInfo.displayName` for a direct chat, shorter expression). + +```diff + // deleteContactOrConversationDialog — line 248 + private fun deleteContactOrConversationDialog(chat: Chat, contact: Contact, chatModel: ChatModel, close: (() -> Unit)?) { + AlertManager.shared.showAlertDialogButtonsColumn( + title = generalGetString(MR.strings.delete_contact_question), ++ text = contact.displayName, + buttons = { /* unchanged */ } + ) + } +``` + +```diff + // deleteActiveContactDialog — line 304 + private fun deleteActiveContactDialog(chat: Chat, contact: Contact, chatModel: ChatModel, close: (() -> Unit)? = null) { + val contactDeleteMode = mutableStateOf(ContactDeleteMode.Full()) + AlertManager.shared.showAlertDialogButtonsColumn( + title = generalGetString(MR.strings.delete_contact_question), +- text = generalGetString(MR.strings.delete_contact_cannot_undo_warning), ++ text = "${contact.displayName}\n\n${generalGetString(MR.strings.delete_contact_cannot_undo_warning)}", + buttons = { /* unchanged */ } + ) + } +``` + +Same diff for `deleteContactWithoutConversation` (line 361) and +`deleteNotReadyContact` (line 417) — both use +`delete_contact_cannot_undo_warning`. Neither takes `contact` as +a parameter, so the name is read via `chat.chatInfo.displayName` +(which resolves to `contact.displayName` because these dispatchers +are only reached for `ChatInfo.Direct` chats). Their titles +(`confirm_delete_contact_question`) stay unchanged — the +not-ready / no-conversation paths keep their distinct title. + +## Commit 2 — iOS + +**Files touched:** +- `apps/ios/Shared/Views/ChatList/ChatListNavLink.swift` +- `apps/ios/Shared/Views/Chat/ChatInfoView.swift` +- `apps/ios/Shared/Views/Chat/Group/GroupChatInfoView.swift` + +### 2.1 — `deleteGroupAlert` (two locations) + +`Views/Chat/Group/GroupChatInfoView.swift:835` and +`Views/ChatList/ChatListNavLink.swift:567` get the same diff. +`deleteGroupAlertMessage(_:)` already returns a `Text` containing +the localized warning — concatenate to it. + +```diff + private func deleteGroupAlert() -> Alert { + let label: LocalizedStringKey = /* unchanged */ + return Alert( + title: Text(label), +- message: deleteGroupAlertMessage(groupInfo), ++ message: Text(chat.chatInfo.displayName) + Text(verbatim: "\n\n") + deleteGroupAlertMessage(groupInfo), + primaryButton: .destructive(Text("Delete")) { /* unchanged */ }, + secondaryButton: .cancel() + ) + } +``` + +`Text(chat.chatInfo.displayName)` resolves to `Text(_ content: some StringProtocol)` +(the runtime-string overload — no localization lookup, matches +codebase convention: `ChatView.swift:984`, `ChatInfoToolbar.swift:49`, +`SettingsView.swift:540`). `Text(verbatim: "\n\n")` is the literal +separator, matching the codebase convention that reserves +`verbatim:` for fixed punctuation (`ContextItemView.swift:88` is +the textbook example: `Text(chatLink.displayName) + Text(verbatim: " - ")`). +The third term `Text(messageLabel)` keeps the existing +`LocalizedStringKey` lookup. + +### 2.2 — `leaveGroupAlert` (two locations) + +`Views/Chat/Group/GroupChatInfoView.swift:872` and +`Views/ChatList/ChatListNavLink.swift:622`: + +```diff + private func leaveGroupAlert() -> Alert { + let titleLabel: LocalizedStringKey = /* unchanged */ + let messageLabel: LocalizedStringKey = /* unchanged */ + return Alert( + title: Text(titleLabel), +- message: Text(messageLabel), ++ message: Text(chat.chatInfo.displayName) + Text(verbatim: "\n\n") + Text(messageLabel), + primaryButton: .destructive(Text("Leave")) { /* unchanged */ }, + secondaryButton: .cancel() + ) + } +``` + +### 2.3 — `clearChatAlert` (three locations) + +`Views/Chat/ChatInfoView.swift:577`, +`Views/Chat/Group/GroupChatInfoView.swift:858`, +`Views/ChatList/ChatListNavLink.swift:600`: + +```diff + private func clearChatAlert() -> Alert { + Alert( + title: Text("Clear conversation?"), +- message: Text("All messages will be deleted - this cannot be undone! The messages will be deleted ONLY for you."), ++ message: Text(chat.chatInfo.displayName) + Text(verbatim: "\n\n") + Text("All messages will be deleted - this cannot be undone! The messages will be deleted ONLY for you."), + primaryButton: .destructive(Text("Clear")) { /* unchanged */ }, + secondaryButton: .cancel() + ) + } +``` + +### 2.4 — Contact-delete action sheets + +Three functions in `Views/Chat/ChatInfoView.swift`. None currently +pass `message:` to `ActionSheet`; we add it. `ActionSheet`'s +`message:` is an optional second parameter that SwiftUI already +supports. + +All three functions have `contact: Contact` in scope. Use bare +`Text(contact.displayName)` (resolves to the `StringProtocol` +overload, no localization lookup, matches codebase convention). +Add only the name as `message:` — these ActionSheets had no +message before, so adding any additional warning would be new +behavior beyond the stated goal. + +**`deleteContactOrConversationDialog`** (line 1177): + +```diff + private func deleteContactOrConversationDialog( + _ chat: Chat, _ contact: Contact, _ dismissToChatList: Bool, + _ showAlert: @escaping (SomeAlert) -> Void, + _ showActionSheet: @escaping (SomeActionSheet) -> Void, + _ showSheetContent: @escaping (SomeSheet) -> Void + ) { + showActionSheet(SomeActionSheet( + actionSheet: ActionSheet( + title: Text("Delete contact?"), ++ message: Text(contact.displayName), + buttons: [ /* unchanged */ ] + ), + id: "deleteContactOrConversationDialog" + )) + } +``` + +**`deleteContactWithoutConversation`** (line 1324): + +```diff + showActionSheet(SomeActionSheet( + actionSheet: ActionSheet( + title: Text("Confirm contact deletion?"), ++ message: Text(contact.displayName), + buttons: [ /* unchanged */ ] + ), + id: "deleteContactWithoutConversation" + )) +``` + +**`deleteNotReadyContact`** (line 1348) — same: + +```diff + showActionSheet(SomeActionSheet( + actionSheet: ActionSheet( + title: Text("Confirm contact deletion?"), ++ message: Text(contact.displayName), + buttons: [ /* unchanged */ ] + ), + id: "deleteNotReadyContact" + )) +``` + +### 2.5 — `DeleteActiveContactDialog` sheet (line 1282) unchanged + +The secondary multi-option sheet is reached only after the user +confirms "Delete contact" in the previous action sheet — which now +shows the name. The sheet itself remains as-is. + +## Verification + +For each platform, exercise every entry point and confirm the +body reads `` on its own line followed by the existing +warning: + +- Android & Desktop: + - Chat list swipe — direct contact, group, channel, business chat + → delete / clear / leave. (Note folder's clear dialog is + intentionally unchanged — `clearNoteFolderDialog` excluded.) + - Chat info screens — "Delete contact" / "Delete group" / "Delete + channel" / "Clear conversation" / "Leave …" rows. + - Contact list (`ContactListNavView.kt:148`) — "Delete contact" + action shows the name in entry-point dialog and toggle dialog. + - Multi-option contact-delete path: entry dialog (now has a name + body where it had none) → toggle dialog (name above the + warning) → success. +- iOS: + - Same matrix from chat list swipe and chat info screens. + - Action-sheet contact-delete paths show the name as the + `message:` line on iPhone and iPad. + +Edge cases: + +- Long chat name — alert containers wrap automatically; the body + occupies 3+ lines. Confirm with a chat renamed to ~40 characters. +- Special characters (emoji, RTL, double quotes) — render literally + via string interpolation, no format-substitution involved. +- Empty `displayName` — does not occur in practice (`NamedChat` + enforces non-empty via `localAlias.ifEmpty { profile.displayName }`). + +Diff-level checks: + +- `git diff '*strings.xml' '*Localizable.strings'` returns zero + hunks. Pure code change. +- `git diff --stat` shows ~5 files total: two Kotlin dispatcher + files, three iOS view files. +- Cancel/confirm flows behave exactly as before — same API calls, + same model updates, same navigation. + +## Out of scope + +- Profile picture / avatar in dialogs — excluded by product decision. +- Refactoring the iOS duplication between `ChatListNavLink` and + `GroupChatInfoView` / `ChatInfoView` (pre-existing `// TODO` at + `GroupChatInfoView.swift:834`). +- Pre-existing wording divergence between Kotlin's "Clear chat?" + and iOS's "Clear conversation?". Both platforms keep their + titles. +- "Delete invitation" at `ChatListNavLink.swift:236` — has no + confirmation dialog (direct call to `deleteChat(chat)`); nothing + to modify. +- Bolding the chat name. SwiftUI `Text + Text` supports `.bold()` + on the first term, but Jetpack Compose `AlertDialog` text is a + single unstyled string — keeping both unstyled preserves parity. diff --git a/plans/delete-leave-dialog-with-profile.md b/plans/delete-leave-dialog-with-profile.md new file mode 100644 index 0000000000..a05fb66532 --- /dev/null +++ b/plans/delete-leave-dialog-with-profile.md @@ -0,0 +1,249 @@ +# Show chat name in delete / leave / clear confirmation dialogs + +## Goal + +The current delete-contact, delete-group, delete-channel, leave-group, +leave-channel and clear-chat confirmations are generic. From a long +chat list, swiping on a row and triggering one of these actions opens +a dialog whose title is "Delete group?", "Leave channel?", "Clear +conversation?" — with no indication of *which* chat is the target. A +user can easily act on the wrong chat. + +The fix: include the chat's display name in the dialog body, on a line +of its own above the existing warning text. Nothing else changes — +same title, same warning text, same buttons, same colors, same dialog +shape. No profile picture, no layout changes, no new helpers, no new +translation strings. + +We deliberately do NOT reuse the open-chat-link alert layout (centered +profile image + name + open-chat button). That layout is the *invite* +flow's identity; repurposing it for destructive confirmations would +confuse the two flows visually. The minimum change that solves the +"which chat?" problem is putting the name in the body text. + +## Why body, not title; why no new strings + +The title carries the action ("Delete group?", "Leave channel?"). The +body carries the consequences ("Group will be deleted for all +members…"). The chat name belongs with the body — it is the subject +of the consequence, not part of the question. + +Adding the name to the title would require new format-string variants +(`delete_group_named_question` etc.) and per-locale re-translation. +Putting the name on its own line in the body is a pure code change — +the existing translated warnings are concatenated with the chat name +in code: + +``` +Tech Talk + +Group will be deleted for all members - this cannot be undone! +``` + +The display name appears first because the user wants to confirm +*which* chat before reading *what* will happen. The blank line between +the name and the warning makes the name visually distinct. + +## Current state + +### Multiplatform (Kotlin / Android / Desktop) + +All eight dialogs go through `AlertManager.shared.showAlertDialog` or +`showAlertDialogButtonsColumn`: + +- `deleteGroupDialog` — `views/chat/group/GroupChatInfoView.kt:182` +- `leaveGroupDialog` — `views/chat/group/GroupChatInfoView.kt:222` +- `clearChatDialog` — `views/chat/ChatInfoView.kt:492` +- `deleteContactOrConversationDialog` — `views/chat/ChatInfoView.kt:248` +- `deleteActiveContactDialog` — `views/chat/ChatInfoView.kt:304` +- `deleteContactWithoutConversation` — `views/chat/ChatInfoView.kt:361` +- `deleteNotReadyContact` — `views/chat/ChatInfoView.kt:417` +- `deleteContactConnectionAlert` — `views/chatlist/ChatListNavLinkView.kt:772` + (deletes a pending contact connection; takes a `PendingContactConnection` + whose `displayName` reflects any custom name the user set) + +Call sites (chat-info screens, chat-list swipe / overflow, contact +list) funnel through these dispatcher functions. + +### iOS (Swift) + +Two SwiftUI patterns are used: + +- SwiftUI `Alert` with `primaryButton: .destructive` / `.cancel()`: + - `deleteGroupAlert` — `Views/ChatList/ChatListNavLink.swift:567`, + `Views/Chat/Group/GroupChatInfoView.swift:835` + - `leaveGroupAlert` — `Views/ChatList/ChatListNavLink.swift:622`, + `Views/Chat/Group/GroupChatInfoView.swift:872` + - `clearChatAlert` — `Views/ChatList/ChatListNavLink.swift:600`, + `Views/Chat/ChatInfoView.swift:577`, + `Views/Chat/Group/GroupChatInfoView.swift:858` +- SwiftUI `ActionSheet`: + - `deleteContactOrConversationDialog` — + `Views/Chat/ChatInfoView.swift:1177` + - `deleteContactWithoutConversation` — + `Views/Chat/ChatInfoView.swift:1324` + - `deleteNotReadyContact` — `Views/Chat/ChatInfoView.swift:1348` + +`Alert(message:)` accepts `Text`, and `ActionSheet(message:)` (an +existing optional parameter not used today) accepts `Text` too — so +the name can be added by composing the existing message string with +`"\n\n"` and the chat name. No widget changes. + +## Design + +| Dialog | Body today | Body after | +|---|---|---| +| Delete group | `Group will be deleted for all members – this cannot be undone!` | `Tech Talk` + blank line + existing text | +| Delete channel | `Channel will be deleted for all subscribers – this cannot be undone!` | `SimpleX news` + blank line + existing text | +| Leave group | `You will stop receiving messages from this group. …` | `Tech Talk` + blank line + existing text | +| Clear chat | `All messages will be deleted – this cannot be undone! …` | `Alice` + blank line + existing text | +| Delete contact (entry sheet) | *(no body today — title only + buttons)* | `Alice` (becomes the body) | +| Delete contact (active variant) | `Contact will be deleted – this cannot be undone!` | `Alice` + blank line + existing text | +| Confirm contact deletion (not-ready / no-conversation) | `Contact will be deleted – this cannot be undone!` | `Alice` + blank line + existing text | + +Title text is unchanged in every case. Existing titles +(`delete_contact_question`, `confirm_delete_contact_question`, etc.) +keep their semantic distinction — the "Confirm contact deletion?" +title still appears for the not-ready / no-conversation paths. + +### Which name to use: `displayName`, not `chatViewName` + +The chat list row labels chats with `cInfo.chatViewName` +(`ChatPreviewView.kt:87`), defined as: + +```kotlin +val chatViewName: String + get() = localAlias.ifEmpty { displayName + (if (fullName == "" || fullName == displayName) "" else " / $fullName") } +``` + +The dialog uses `chatInfo.displayName` (and `groupInfo.displayName` +for the leave dialog). For most chats these are identical: + +- If `localAlias` is set, both resolve to the alias. +- If `displayName == fullName` (or `fullName` is empty), both resolve + to `displayName`. + +For a contact with distinct display name and full name (no alias), +the row would show `alice / Alice Smith` while the dialog shows +`alice`. Acceptable: `displayName` is the recognizable identifier, +shorter, and the dialog format (single line above the warning) +benefits from concision. Two-part identifiers in the dialog would +crowd the layout. + +### `clearNoteFolderDialog` is excluded + +The local notes folder is a single-instance object — there is only +one per user — and its existing warning text already names it +unambiguously. Adding the display name on its own line would be +pure redundancy. Skipped. + +## Changes + +### Multiplatform (Kotlin) + +Each dispatcher function changes one argument: the `text =` parameter +passed to `AlertManager.shared.showAlertDialog` / +`showAlertDialogButtonsColumn`. The new value is the chat name + two +newlines + the existing message text: + +```kotlin +text = "${chatInfo.displayName}\n\n${generalGetString(messageId)}", +parseHtml = false, +``` + +`parseHtml = false` is a new boolean parameter added to both alert +helpers. It bypasses `escapedHtmlToAnnotatedString` so the +user-controlled `displayName` is rendered as literal text, never +interpreted as HTML markup (``, ``, `&`, etc.). The default +remains `true`; only our delete-confirmation dispatchers opt out. + +For `leaveGroupDialog` the source is `groupInfo.displayName` (the +function already takes `groupInfo` — no signature change needed, +no caller updates needed). + +For `deleteGroupDialog`, also `groupInfo.displayName`, for consistency +with `leaveGroupDialog` (both have `groupInfo` already in scope). + +For `deleteContactOrConversationDialog`, which has no `text =` +parameter today, add `text = chatInfo.displayName` (no concatenation +needed — the dialog had no body text before). + +### iOS + +Each of the eight call sites changes one argument: the `message:` +parameter passed to `Alert(…)` or `ActionSheet(…)`. The new value +composes the chat name with the existing localized message string: + +```swift +message: Text("\(chat.chatInfo.displayName)\n\n\(existingMessage)"), +``` + +For the three `ActionSheet` sites that have no `message:` today, add +`message: Text(chat.chatInfo.displayName)`. + +## Out of scope + +- Profile picture / avatar in any of these dialogs — excluded by + decision: the open-chat-link alert owns that layout, and reusing + it for destructive confirmations conflates two semantically + different flows. +- The pre-existing wording divergence between Kotlin's + `clear_chat_question` ("Clear chat?") and iOS's "Clear + conversation?". Both platforms keep their existing titles. +- Refactoring the iOS duplication between `ChatListNavLink` and + `GroupChatInfoView` / `ChatInfoView` (pre-existing `// TODO reuse + this and clearChatAlert with ChatInfoView` at + `GroupChatInfoView.swift:834`). +- "Delete invitation" at `ChatListNavLink.swift:236` — goes through + `deleteChat(chat)` directly with no confirmation dialog. No dialog + to modify. +- Bolding the chat name on its own line. SwiftUI `Text` concatenation + supports `.bold()`; Jetpack Compose `AlertDialog` text is a single + string. Keep both platforms unstyled for parity. + +## Verification + +Per platform, exercise every entry point and confirm the dialog body +reads `` on its own line followed by a blank line followed +by the existing warning: + +- Android & Desktop: + - Chat list swipe — direct contact, group, channel, business chat + → delete / clear / leave actions. (Note folder's clear dialog + is intentionally unchanged.) + - Chat info screens — "Delete contact" / "Delete group" / "Delete + channel" / "Clear conversation" / "Leave …" rows. + - Contact list (`ContactListNavView.kt:148`) — "Delete contact" + action. + - The multi-option contact-delete path: entry dialog (now has a + name body where it had none) → toggle dialog (name above the + warning) → success. +- iOS: + - Same matrix from chat list swipe and chat info screens. + - Action-sheet contact-delete paths show the name as the + `message:` line. + +Edge cases: + +- Long chat name (40+ chars) — alert containers wrap automatically; + body now occupies 3+ lines (name on 2, blank line, warning on 1+). + Confirm via a chat renamed to a long string. +- Special characters in name (emoji, RTL text, double quotes) — + render literally because the substitution is string concatenation, + not format expansion. A contact named `Bob "the builder"` displays + as `Bob "the builder"` on its own line. No quoting/escaping issue. +- Empty `displayName` would render an empty first line above the + warning. In practice `displayName` is non-empty (the `NamedChat` + interface enforces it via `localAlias.ifEmpty { profile.displayName }`); + no defensive trimming added. + +Diff-level checks: + +- `git diff strings.xml` and `git diff '*Localizable.strings'` show + zero hunks. The change is pure code. +- `git diff --stat` shows each platform touched in 2–4 files: + the dispatcher file(s) on Kotlin (`ChatInfoView.kt`, + `GroupChatInfoView.kt`), and the SwiftUI views holding the + alert/sheet builders on iOS. +- Behavior is unchanged. Cancel returns to the prior screen; + confirm performs the same destructive API call as before. diff --git a/src/Simplex/Chat/Library/Commands.hs b/src/Simplex/Chat/Library/Commands.hs index 6b54019775..0dd312abb2 100644 --- a/src/Simplex/Chat/Library/Commands.hs +++ b/src/Simplex/Chat/Library/Commands.hs @@ -652,12 +652,14 @@ processChatCommand cxt nm = \case _ <- createChatTag db user emoji text CRChatTags user <$> getUserChatTags db user APISetChatTags (ChatRef cType chatId scope) tagIds -> withUser $ \user -> case cType of - CTDirect -> withFastStore' $ \db -> do - updateDirectChatTags db chatId (maybe [] L.toList tagIds) - CRTagsUpdated user <$> getUserChatTags db user <*> getDirectChatTags db chatId - CTGroup | isNothing scope -> withFastStore' $ \db -> do - updateGroupChatTags db chatId (maybe [] L.toList tagIds) - CRTagsUpdated user <$> getUserChatTags db user <*> getGroupChatTags db chatId + CTDirect -> withFastStore $ \db -> do + Contact {contactId} <- getContact db cxt user chatId + liftIO $ updateDirectChatTags db contactId (maybe [] L.toList tagIds) + CRTagsUpdated user <$> liftIO (getUserChatTags db user) <*> liftIO (getDirectChatTags db contactId) + CTGroup | isNothing scope -> withFastStore $ \db -> do + GroupInfo {groupId} <- getGroupInfo db cxt user chatId + liftIO $ updateGroupChatTags db groupId (maybe [] L.toList tagIds) + CRTagsUpdated user <$> liftIO (getUserChatTags db user) <*> liftIO (getGroupChatTags db groupId) _ -> throwCmdError "not supported" APIDeleteChatTag tagId -> withUser $ \user -> do withFastStore' $ \db -> deleteChatTag db user tagId @@ -1694,8 +1696,11 @@ processChatCommand cxt nm = \case CRServerOperatorConditions <$> getServerOperators db APISetChatTTL userId (ChatRef cType chatId scope) newTTL_ -> withUserId userId $ \user -> checkStoreNotChanged $ withChatLock "setChatTTL" $ do - (oldTTL_, globalTTL, ttlCount) <- withStore' $ \db -> - (,,) <$> getSetChatTTL db <*> getChatItemTTL db user <*> getChatTTLCount db user + (oldTTL_, globalTTL, ttlCount) <- withStore $ \db -> do + oldTTL <- getSetChatTTL db user + globalTTL <- liftIO $ getChatItemTTL db user + ttlCount <- liftIO $ getChatTTLCount db user + pure (oldTTL, globalTTL, ttlCount) let newTTL = fromMaybe globalTTL newTTL_ oldTTL = fromMaybe globalTTL oldTTL_ when (newTTL > 0 && (newTTL < oldTTL || oldTTL == 0)) $ do @@ -1704,9 +1709,13 @@ processChatCommand cxt nm = \case lift $ setChatItemsExpiration user globalTTL ttlCount ok user where - getSetChatTTL db = case cType of - CTDirect -> getDirectChatTTL db chatId <* setDirectChatTTL db chatId newTTL_ - CTGroup | isNothing scope -> getGroupChatTTL db chatId <* setGroupChatTTL db chatId newTTL_ + getSetChatTTL db currentUser = case cType of + CTDirect -> do + Contact {contactId} <- getContact db cxt currentUser chatId + liftIO $ getDirectChatTTL db contactId <* setDirectChatTTL db contactId newTTL_ + CTGroup | isNothing scope -> do + GroupInfo {groupId} <- getGroupInfo db cxt currentUser chatId + liftIO $ getGroupChatTTL db groupId <* setGroupChatTTL db groupId newTTL_ _ -> pure Nothing expireChat user globalTTL = do currentTs <- liftIO getCurrentTime diff --git a/src/Simplex/Chat/Library/Subscriber.hs b/src/Simplex/Chat/Library/Subscriber.hs index 46174d631c..88e9c283af 100644 --- a/src/Simplex/Chat/Library/Subscriber.hs +++ b/src/Simplex/Chat/Library/Subscriber.hs @@ -3919,13 +3919,13 @@ runDeliveryTaskWorker a deliveryKey Worker {doWork} = do withStore' $ \db -> setDeliveryTaskErrStatus db (deliveryTaskId task) "relay inactive" | otherwise -> withWorkItems a doWork (withStore' $ \db -> getNextDeliveryTasks db gInfo task) $ \nextTasks -> do - let (body, acceptedTasks, largeTasks) = batchDeliveryTasks1 (vr cxt) maxEncodedMsgLength nextTasks + let (body_, acceptedTasks, largeTasks) = batchDeliveryTasks1 (vr cxt) maxEncodedMsgLength nextTasks senderGMIds = S.toList . S.fromList $ map (\MessageDeliveryTask {senderGMId} -> senderGMId) acceptedTasks withStore' $ \db -> do - createMsgDeliveryJob db gInfo jobScope senderGMIds body + forM_ body_ $ \body -> createMsgDeliveryJob db gInfo jobScope senderGMIds body forM_ acceptedTasks $ \t -> updateDeliveryTaskStatus db (deliveryTaskId t) DTSProcessed forM_ largeTasks $ \t -> setDeliveryTaskErrStatus db (deliveryTaskId t) "large" - lift . void $ getDeliveryJobWorker True deliveryKey + when (isJust body_) . lift . void $ getDeliveryJobWorker True deliveryKey -- DJRelayRemoved is allowed when RSInactive - it forwards XGrpMemDel about relay's own deletion DJRelayRemoved | workerScope /= DWSGroup -> diff --git a/src/Simplex/Chat/Messages/Batch.hs b/src/Simplex/Chat/Messages/Batch.hs index ed65bd4af7..81861aad74 100644 --- a/src/Simplex/Chat/Messages/Batch.hs +++ b/src/Simplex/Chat/Messages/Batch.hs @@ -24,7 +24,6 @@ import qualified Data.ByteString as BS import qualified Data.ByteString.Char8 as B import Data.Char (ord) import Data.Function (on) -import Data.Foldable (foldr') import Data.List (foldl', sortBy) import Data.List.NonEmpty (NonEmpty (..)) import qualified Data.List.NonEmpty as L @@ -79,15 +78,15 @@ batchMessages mode maxLen = addBatch . foldr addToBatch ([], [], [], 0, 0) let encoded = encodeBatch mode bodies in Right (MsgBatch encoded msgs) : batches --- | Batches delivery tasks into (batch, accepted, large). +-- | Batches delivery tasks into (batch if any task was accepted, accepted, large). -- Always uses binary batch format for relay groups. -batchDeliveryTasks1 :: VersionRangeChat -> Int -> NonEmpty MessageDeliveryTask -> (ByteString, [MessageDeliveryTask], [MessageDeliveryTask]) +batchDeliveryTasks1 :: VersionRangeChat -> Int -> NonEmpty MessageDeliveryTask -> (Maybe ByteString, [MessageDeliveryTask], [MessageDeliveryTask]) batchDeliveryTasks1 _vr maxLen = toResult . foldl' addToBatch ([], [], [], 0, 0) . L.toList where addToBatch :: ([ByteString], [MessageDeliveryTask], [MessageDeliveryTask], Int, Int) -> MessageDeliveryTask -> ([ByteString], [MessageDeliveryTask], [MessageDeliveryTask], Int, Int) addToBatch (msgBodies, accepted, large, len, n) task - -- too large: skip, record in large - | msgLen > maxLen = (msgBodies, accepted, task : large, len, n) + -- element can't fit even a singleton batch (4-byte binary-batch framing) + | msgLen + 4 > maxLen = (msgBodies, accepted, task : large, len, n) -- fits: include in batch -- batch overhead: '=' + count (2) + 2-byte length prefix per element | len' + (n + 1) * 2 + 2 <= maxLen = (msgBody : msgBodies, task : accepted, large, len', n + 1) @@ -98,10 +97,11 @@ batchDeliveryTasks1 _vr maxLen = toResult . foldl' addToBatch ([], [], [], 0, 0) msgBody = encodeFwdElement GrpMsgForward {fwdSender, fwdBrokerTs} verifiedMsg msgLen = B.length msgBody len' = len + msgLen - toResult :: ([ByteString], [MessageDeliveryTask], [MessageDeliveryTask], Int, Int) -> (ByteString, [MessageDeliveryTask], [MessageDeliveryTask]) + toResult :: ([ByteString], [MessageDeliveryTask], [MessageDeliveryTask], Int, Int) -> (Maybe ByteString, [MessageDeliveryTask], [MessageDeliveryTask]) toResult (msgBodies, accepted, large, _, _) = let encoded = encodeBinaryBatch (reverse msgBodies) - in (encoded, reverse accepted, reverse large) + body = if null accepted then Nothing else Just encoded + in (body, reverse accepted, reverse large) -- | Encode a batch element for relay groups: >[/]. encodeFwdElement :: GrpMsgForward -> VerifiedMsg 'Json -> ByteString diff --git a/src/Simplex/Chat/Store/Files.hs b/src/Simplex/Chat/Store/Files.hs index 2db63f6a5b..2c2475a9ba 100644 --- a/src/Simplex/Chat/Store/Files.hs +++ b/src/Simplex/Chat/Store/Files.hs @@ -83,6 +83,7 @@ import Data.Functor ((<&>)) import Data.Int (Int64) import Data.Maybe (fromMaybe, isJust, listToMaybe) import Data.Text (Text) +import qualified Data.Text as T import Data.Time (addUTCTime) import Data.Time.Clock (UTCTime (..), getCurrentTime, nominalDay) import Data.Type.Equality @@ -462,7 +463,7 @@ createRcvStandaloneFileTransfer db userId (CryptoFile filePath cfArgs_) fileSize createRcvFD_ :: DB.Connection -> UserId -> UTCTime -> FileDescr -> ExceptT StoreError IO RcvFileDescr createRcvFD_ db userId currentTs FileDescr {fileDescrText, fileDescrPartNo, fileDescrComplete} = do - when (fileDescrPartNo /= 0) $ throwError SERcvFileInvalidDescrPart + when (fileDescrPartNo /= 0 || not (rcvFileDescrWithinLimits fileDescrPartNo fileDescrText)) $ throwError SERcvFileInvalidDescrPart fileDescrId <- liftIO $ do DB.execute db @@ -490,8 +491,8 @@ appendRcvFD db userId fileId fd@FileDescr {fileDescrText, fileDescrPartNo, fileD fileDescrPartNo = rfdPNo, fileDescrComplete = rfdComplete } -> do - when (fileDescrPartNo /= rfdPNo + 1 || rfdComplete) $ throwError SERcvFileInvalidDescrPart let fileDescrText' = rfdText <> fileDescrText + when (fileDescrPartNo /= rfdPNo + 1 || rfdComplete || not (rcvFileDescrWithinLimits fileDescrPartNo fileDescrText')) $ throwError SERcvFileInvalidDescrPart liftIO $ DB.execute db @@ -503,6 +504,23 @@ appendRcvFD db userId fileId fd@FileDescr {fileDescrText, fileDescrPartNo, fileD (fileDescrText', fileDescrPartNo, BI fileDescrComplete, fileDescrId) pure RcvFileDescr {fileDescrId, fileDescrText = fileDescrText', fileDescrPartNo, fileDescrComplete} +-- Upper bounds sized above the largest legitimate received description; derived from simplexmq's +-- chunk tiers and redundancy, so a change there must revisit them. +-- ~1280 chunks max = maxFileSizeHard (5gb) / largest chunk tier (4mb). +-- ~150 chars per chunk in the description YAML = replicaId 24 + Ed25519 key 64 + SHA-256 digest 44 + chunkNo/colons. +-- Total ~0.18 MB at 1 replica/chunk (~0.42 MB at 3x), under the 1mb text and 1024 part caps. +maxRcvFileDescrParts :: Int +maxRcvFileDescrParts = 1024 + +maxRcvFileDescrTextLength :: Int +maxRcvFileDescrTextLength = 1024 * 1024 + +rcvFileDescrWithinLimits :: Int -> Text -> Bool +rcvFileDescrWithinLimits partNo descrText = + partNo >= 0 + && partNo <= maxRcvFileDescrParts + && T.length descrText <= maxRcvFileDescrTextLength + getRcvFileDescrByRcvFileId :: DB.Connection -> FileTransferId -> ExceptT StoreError IO RcvFileDescr getRcvFileDescrByRcvFileId db fileId = do liftIO (getRcvFileDescrByRcvFileId_ db fileId) >>= \case diff --git a/tests/ChatTests/Direct.hs b/tests/ChatTests/Direct.hs index 740e757ed8..7acfae1a95 100644 --- a/tests/ChatTests/Direct.hs +++ b/tests/ChatTests/Direct.hs @@ -122,6 +122,7 @@ chatDirectTests = do it "create user with same servers" testCreateUserSameServers it "delete user" testDeleteUser it "delete user with chat tags" testDeleteUserChatTags + it "rejects raw chat TTL updates for another user's chat" testRejectCrossUserChatTTL it "users have different chat item TTL configuration, chat items expire" testUsersDifferentCIExpirationTTL it "chat items expire after restart for all users according to per user configuration" testUsersRestartCIExpiration it "chat items only expire for users who configured expiration" testEnableCIExpirationOnlyForOneUser @@ -2096,6 +2097,25 @@ testDeleteUserChatTags = alice ##> "/users" alice <## "alisa (active)" +testRejectCrossUserChatTTL :: HasCallStack => TestParams -> IO () +testRejectCrossUserChatTTL = + testChat2 aliceProfile bobProfile $ + \alice bob -> do + connectUsers alice bob + + alice #$> ("/_ttl 1 @2 2", id, "ok") + alice #$> ("/ttl @bob", id, "old messages are set to be deleted after: 2 second(s)") + + alice ##> "/create user alisa" + showActiveUser alice "alisa" + + alice ##> "/_ttl 2 @2 9" + alice <##. "chat db error:" + + alice ##> "/user alice" + showActiveUser alice "alice (Alice)" + alice #$> ("/ttl @bob", id, "old messages are set to be deleted after: 2 second(s)") + testUsersDifferentCIExpirationTTL :: HasCallStack => TestParams -> IO () testUsersDifferentCIExpirationTTL ps = do withNewTestChat ps "bob" bobProfile $ \bob -> do diff --git a/tests/MessageBatching.hs b/tests/MessageBatching.hs index 05322a0834..00cbbd757b 100644 --- a/tests/MessageBatching.hs +++ b/tests/MessageBatching.hs @@ -12,14 +12,31 @@ import qualified Data.ByteString as B import Data.ByteString.Internal (c2w) import Data.Either (partitionEithers) import Data.Int (Int64) +import Data.List.NonEmpty (NonEmpty (..)) import Data.String (IsString (..)) import qualified Data.Text as T import Data.Text.Encoding (encodeUtf8) +import Data.Time.Clock.System (SystemTime (..), systemToUTCTime) +import Simplex.Chat.Delivery + ( DeliveryJobScope (DJSGroup, jobSpec), + DeliveryJobSpec (DJDeliveryJob, includePending), + MessageDeliveryTask (MessageDeliveryTask, brokerTs, fwdSender, jobScope, senderGMId, taskId, verifiedMsg), + deliveryTaskId, + ) import Simplex.Chat.Messages.Batch import Simplex.Chat.Controller (ChatError (..), ChatErrorType (..)) import Simplex.Chat.Messages (SndMessage (..)) -import Simplex.Chat.Protocol (maxEncodedMsgLength) -import Simplex.Chat.Types (SharedMsgId (..)) +import Simplex.Chat.Protocol + ( ChatMessage (ChatMessage), + ChatMsgEvent (XMsgNew), + FwdSender (FwdChannel), + GrpMsgForward (GrpMsgForward), + MsgContent (MCText), + VerifiedMsg (VMUnsigned), + maxEncodedMsgLength, + mcSimple, + ) +import Simplex.Chat.Types (SharedMsgId (..), chatInitialVRange) import Simplex.Messaging.Encoding (Large (..), smpEncodeList) import Test.Hspec @@ -28,6 +45,8 @@ batchingTests = describe "message batching tests" $ do testBatchingCorrectness testBinaryBatchingCorrectness it "image x.msg.new and x.msg.file.descr should fit into single batch" testImageFitsSingleBatch + it "does not create a relay delivery body when every task is oversized" testRelayBatchAllLarge + it "classifies a task that fits raw but not as a framed singleton as large" testRelayBatchSingletonOverflow instance IsString SndMessage where fromString s = SndMessage {msgId, sharedMsgId = SharedMsgId "", msgBody = s', signedMsg_ = Nothing} @@ -131,6 +150,37 @@ testImageFitsSingleBatch = do runBatcherTest' BMJson maxEncodedMsgLength [msg xMsgNewStr, msg descrStr] [] [batched] +testRelayBatchAllLarge :: IO () +testRelayBatchAllLarge = do + let task1 = deliveryTask 1 "one" + task2 = deliveryTask 2 "two" + (body_, accepted, large) = batchDeliveryTasks1 chatInitialVRange 1 (task1 :| [task2]) + body_ `shouldBe` Nothing + map deliveryTaskId accepted `shouldBe` [] + map deliveryTaskId large `shouldBe` [1, 2] + +deliveryTask :: Int64 -> T.Text -> MessageDeliveryTask +deliveryTask taskId text = + MessageDeliveryTask + { taskId, + jobScope = DJSGroup {jobSpec = DJDeliveryJob {includePending = False}}, + senderGMId = 1, + fwdSender = FwdChannel, + brokerTs = systemToUTCTime $ MkSystemTime 0 0, + verifiedMsg = + VMUnsigned + (ChatMessage chatInitialVRange Nothing $ XMsgNew $ mcSimple $ MCText text) + } + +testRelayBatchSingletonOverflow :: IO () +testRelayBatchSingletonOverflow = do + let task = deliveryTask 1 "overflow" + elemLen = B.length $ encodeFwdElement (GrpMsgForward (fwdSender task) (brokerTs task)) (verifiedMsg task) + (body_, accepted, large) = batchDeliveryTasks1 chatInitialVRange (elemLen + 2) (task :| []) + body_ `shouldBe` Nothing + map deliveryTaskId accepted `shouldBe` [] + map deliveryTaskId large `shouldBe` [1] + runBatcherTest :: BatchMode -> Int -> [SndMessage] -> [ChatError] -> [ByteString] -> Spec runBatcherTest mode maxLen msgs expectedErrors expectedBatches = it