mirror of
https://github.com/permissionlesstech/bitchat.git
synced 2026-08-29 07:27:16 +00:00
Make DeliveryStatus non-optional with an explicit .notSentYet state (#1503)
BitchatMessage.deliveryStatus was Optional, with nil implicitly meaning 'no tracking' for public messages. Every consumer had to branch on the absent case, ranking needed an optional-aware helper, and the UI treated nil as an invisible state (#644). Model delivery as a total state machine instead: - New DeliveryStatus.notSentYet: created but not yet handed to any transport. Public messages initialize to it; private messages keep their historical .sending default. - BitchatMessage.deliveryStatus becomes non-optional. Archives written while the field was optional decode with the absent key mapped to .notSentYet. The wire format is untouched (toBinaryPayload never carried the field). - deliveryStatusRank drops its optional parameter; .notSentYet ranks below .failed, preserving the existing dedup preference order. - Conversation.shouldSkipStatusUpdate treats a write back to .notSentYet as a downgrade and skips it. - The status indicator renders exactly as before: .notSentYet draws nothing in message rows (the state nil used to represent), and DeliveryStatusView gains a glyph and description for it only so the view stays total. Tests: initialization defaults, legacy-archive decoding, round-trip, the extended rank order, and the new downgrade rule. Fixes #644
This commit is contained in:
parent
ab835e58c9
commit
81837d7202
@ -230,8 +230,7 @@ final class Conversation: ObservableObject, Identifiable {
|
||||
|
||||
// MARK: Internals
|
||||
|
||||
static func shouldSkipStatusUpdate(current: DeliveryStatus?, new: DeliveryStatus) -> Bool {
|
||||
guard let current else { return false }
|
||||
static func shouldSkipStatusUpdate(current: DeliveryStatus, new: DeliveryStatus) -> Bool {
|
||||
if current == new { return true }
|
||||
|
||||
// Never downgrade to a weaker delivery state. Ordering of certainty:
|
||||
@ -254,6 +253,10 @@ final class Conversation: ObservableObject, Identifiable {
|
||||
return true
|
||||
case (.sent, .sending):
|
||||
return true
|
||||
case (_, .notSentYet):
|
||||
// .notSentYet is the pre-transport initial state; once a message
|
||||
// has any real status, resetting to it is always a downgrade.
|
||||
return true
|
||||
default:
|
||||
return false
|
||||
}
|
||||
|
||||
@ -203,14 +203,12 @@ final class PrivateChatManager: ObservableObject {
|
||||
func syncReadReceiptsForSentMessages(peerID: PeerID, nickname: String, externalReceipts: inout Set<String>) {
|
||||
for message in messages(for: peerID) {
|
||||
if message.sender == nickname {
|
||||
if let status = message.deliveryStatus {
|
||||
switch status {
|
||||
case .read, .delivered:
|
||||
externalReceipts.insert(message.id)
|
||||
sentReadReceipts.insert(message.id)
|
||||
case .failed, .partiallyDelivered, .sending, .sent, .carried:
|
||||
break
|
||||
}
|
||||
switch message.deliveryStatus {
|
||||
case .read, .delivered:
|
||||
externalReceipts.insert(message.id)
|
||||
sentReadReceipts.insert(message.id)
|
||||
case .notSentYet, .failed, .partiallyDelivered, .sending, .sent, .carried:
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -360,9 +360,9 @@ private extension ChatLifecycleCoordinator {
|
||||
}
|
||||
}
|
||||
|
||||
func deliveryStatusRank(_ status: DeliveryStatus?) -> Int {
|
||||
guard let status else { return 0 }
|
||||
func deliveryStatusRank(_ status: DeliveryStatus) -> Int {
|
||||
switch status {
|
||||
case .notSentYet: return 0
|
||||
case .failed: return 1
|
||||
case .sending: return 2
|
||||
case .sent: return 3
|
||||
|
||||
@ -15,6 +15,8 @@ extension DeliveryStatus {
|
||||
/// the glyphs alone are unexplained 10pt icons.
|
||||
var bitchatDescription: String {
|
||||
switch self {
|
||||
case .notSentYet:
|
||||
return String(localized: "content.delivery.not_sent_yet", defaultValue: "Not sent yet", comment: "Delivery status description for a message that has not entered any send pipeline")
|
||||
case .sending:
|
||||
return String(localized: "content.delivery.sending", comment: "Delivery status description while a private message is being sent")
|
||||
case .sent:
|
||||
@ -72,6 +74,13 @@ struct DeliveryStatusView: View {
|
||||
@ViewBuilder
|
||||
private var statusGlyph: some View {
|
||||
switch status {
|
||||
case .notSentYet:
|
||||
// Normally hidden by callers; shown as a hollow dotted circle if
|
||||
// it ever surfaces so the state is visible rather than invisible.
|
||||
Image(systemName: "circle.dotted")
|
||||
.font(.bitchatSystem(size: 10))
|
||||
.foregroundColor(secondaryTextColor.opacity(0.6))
|
||||
|
||||
case .sending:
|
||||
Image(systemName: "circle")
|
||||
.font(.bitchatSystem(size: 10))
|
||||
@ -125,6 +134,7 @@ struct DeliveryStatusView: View {
|
||||
|
||||
#Preview {
|
||||
let statuses: [DeliveryStatus] = [
|
||||
.notSentYet,
|
||||
.sending,
|
||||
.sent,
|
||||
.carried,
|
||||
|
||||
@ -23,7 +23,7 @@ struct TextMessageView: View {
|
||||
/// SAME instance would otherwise compare "unchanged" and this row's body
|
||||
/// would be skipped even though the parent list re-rendered. Snapshotting
|
||||
/// the enum makes the change visible to SwiftUI's structural diff.
|
||||
private let deliveryStatus: DeliveryStatus?
|
||||
private let deliveryStatus: DeliveryStatus
|
||||
@State private var expandedMessageIDs: Set<String> = []
|
||||
@State private var showDeliveryDetail = false
|
||||
|
||||
@ -68,11 +68,11 @@ struct TextMessageView: View {
|
||||
// .help() tooltips only exist on macOS, so iOS users get the
|
||||
// explanation as a caption under the row instead.
|
||||
if message.isPrivate && conversationUIModel.isSentByCurrentUser(message),
|
||||
let status = deliveryStatus {
|
||||
deliveryStatus != .notSentYet {
|
||||
Button {
|
||||
showDeliveryDetail.toggle()
|
||||
} label: {
|
||||
DeliveryStatusView(status: status)
|
||||
DeliveryStatusView(status: deliveryStatus)
|
||||
.padding(.leading, 4)
|
||||
.contentShape(Rectangle())
|
||||
}
|
||||
@ -86,15 +86,15 @@ struct TextMessageView: View {
|
||||
// Failure reasons stay visible without a tap; other statuses
|
||||
// reveal on demand.
|
||||
if message.isPrivate && conversationUIModel.isSentByCurrentUser(message),
|
||||
let status = deliveryStatus {
|
||||
if case .failed = status {
|
||||
Text(verbatim: status.bitchatDescription)
|
||||
deliveryStatus != .notSentYet {
|
||||
if case .failed = deliveryStatus {
|
||||
Text(verbatim: deliveryStatus.bitchatDescription)
|
||||
.bitchatFont(size: 11)
|
||||
.foregroundColor(Color.red.opacity(0.9))
|
||||
.fixedSize(horizontal: false, vertical: true)
|
||||
.padding(.top, 2)
|
||||
} else if showDeliveryDetail {
|
||||
Text(verbatim: status.bitchatDescription)
|
||||
Text(verbatim: deliveryStatus.bitchatDescription)
|
||||
.bitchatFont(size: 11)
|
||||
.foregroundColor(palette.secondary)
|
||||
.fixedSize(horizontal: false, vertical: true)
|
||||
|
||||
@ -20,7 +20,7 @@ struct MediaMessageView: View {
|
||||
/// is a reference type mutated in place, and SwiftUI compares reference
|
||||
/// fields by identity, so without the snapshot a status-only change
|
||||
/// (send progress, delivered → read) would not re-render this row.
|
||||
private let deliveryStatus: DeliveryStatus?
|
||||
private let deliveryStatus: DeliveryStatus
|
||||
@State private var showDeliveryDetail = false
|
||||
|
||||
@Binding var imagePreviewURL: URL?
|
||||
@ -57,11 +57,11 @@ struct MediaMessageView: View {
|
||||
// .help() tooltips only exist on macOS, so iOS users get the
|
||||
// explanation as a caption under the row instead.
|
||||
if message.isPrivate && conversationUIModel.isSentByCurrentUser(message),
|
||||
let status = deliveryStatus {
|
||||
deliveryStatus != .notSentYet {
|
||||
Button {
|
||||
showDeliveryDetail.toggle()
|
||||
} label: {
|
||||
DeliveryStatusView(status: status)
|
||||
DeliveryStatusView(status: deliveryStatus)
|
||||
.padding(.leading, 4)
|
||||
.contentShape(Rectangle())
|
||||
}
|
||||
@ -75,14 +75,14 @@ struct MediaMessageView: View {
|
||||
// Failure reasons stay visible without a tap; other statuses
|
||||
// reveal on demand.
|
||||
if message.isPrivate && conversationUIModel.isSentByCurrentUser(message),
|
||||
let status = deliveryStatus {
|
||||
if case .failed = status {
|
||||
Text(verbatim: status.bitchatDescription)
|
||||
deliveryStatus != .notSentYet {
|
||||
if case .failed = deliveryStatus {
|
||||
Text(verbatim: deliveryStatus.bitchatDescription)
|
||||
.bitchatFont(size: 11)
|
||||
.foregroundColor(Color.red.opacity(0.9))
|
||||
.fixedSize(horizontal: false, vertical: true)
|
||||
} else if showDeliveryDetail {
|
||||
Text(verbatim: status.bitchatDescription)
|
||||
Text(verbatim: deliveryStatus.bitchatDescription)
|
||||
.bitchatFont(size: 11)
|
||||
.foregroundColor(palette.secondary)
|
||||
.fixedSize(horizontal: false, vertical: true)
|
||||
@ -132,26 +132,24 @@ struct MediaMessageView: View {
|
||||
}
|
||||
}
|
||||
|
||||
private func mediaSendState(for deliveryStatus: DeliveryStatus?, isFromMe: Bool) -> (isSending: Bool, progress: Double?, canCancel: Bool) {
|
||||
private func mediaSendState(for deliveryStatus: DeliveryStatus, isFromMe: Bool) -> (isSending: Bool, progress: Double?, canCancel: Bool) {
|
||||
// A received message is never in a send state: BitchatMessage defaults
|
||||
// private messages to .sending, so an incoming message's status must
|
||||
// not drive the reveal mask or disable the reveal tap.
|
||||
guard isFromMe else { return (false, nil, false) }
|
||||
var isSending = false
|
||||
var progress: Double?
|
||||
if let status = deliveryStatus {
|
||||
switch status {
|
||||
case .sending:
|
||||
switch deliveryStatus {
|
||||
case .sending:
|
||||
isSending = true
|
||||
progress = 0
|
||||
case .partiallyDelivered(let reached, let total):
|
||||
if total > 0 {
|
||||
isSending = true
|
||||
progress = 0
|
||||
case .partiallyDelivered(let reached, let total):
|
||||
if total > 0 {
|
||||
isSending = true
|
||||
progress = Double(reached) / Double(total)
|
||||
}
|
||||
case .sent, .carried, .read, .delivered, .failed:
|
||||
break
|
||||
progress = Double(reached) / Double(total)
|
||||
}
|
||||
case .notSentYet, .sent, .carried, .read, .delivered, .failed:
|
||||
break
|
||||
}
|
||||
let canCancel = isSending && conversationUIModel.isSentByCurrentUser(message)
|
||||
let clamped = progress.map { max(0, min(1, $0)) }
|
||||
|
||||
@ -430,7 +430,7 @@ private extension MessageListView {
|
||||
guard message.isPrivate,
|
||||
conversationUIModel.isSentByCurrentUser(message),
|
||||
conversationUIModel.mediaAttachment(for: message) == nil,
|
||||
case .some(.failed) = message.deliveryStatus
|
||||
case .failed = message.deliveryStatus
|
||||
else { return false }
|
||||
return true
|
||||
}
|
||||
|
||||
@ -147,6 +147,10 @@ struct ChatViewModelDeliveryStatusTests {
|
||||
#expect(Conversation.shouldSkipStatusUpdate(current: .sent, new: .sending))
|
||||
// ...but a retry after a real failure stays visible.
|
||||
#expect(!Conversation.shouldSkipStatusUpdate(current: .failed(reason: "no route"), new: .sending))
|
||||
// .notSentYet is the pre-transport initial state: leaving it is always
|
||||
// allowed, returning to it never is.
|
||||
#expect(!Conversation.shouldSkipStatusUpdate(current: .notSentYet, new: .sending))
|
||||
#expect(Conversation.shouldSkipStatusUpdate(current: .sent, new: .notSentYet))
|
||||
}
|
||||
|
||||
@Test @MainActor
|
||||
@ -729,9 +733,10 @@ struct ChatViewModelDeliveryStatusTests {
|
||||
@Test @MainActor
|
||||
func statusRank_orderingIsCorrect() async {
|
||||
// This tests the implicit ordering used in refreshVisibleMessages
|
||||
// failed < sending < sent < carried < partiallyDelivered < delivered < read
|
||||
// notSentYet < failed < sending < sent < carried < partiallyDelivered < delivered < read
|
||||
|
||||
let statuses: [DeliveryStatus] = [
|
||||
.notSentYet,
|
||||
.failed(reason: "test"),
|
||||
.sending,
|
||||
.sent,
|
||||
@ -745,13 +750,14 @@ struct ChatViewModelDeliveryStatusTests {
|
||||
// This is more of a documentation test to ensure the ranking logic is understood
|
||||
for (index, status) in statuses.enumerated() {
|
||||
switch status {
|
||||
case .failed: #expect(index == 0)
|
||||
case .sending: #expect(index == 1)
|
||||
case .sent: #expect(index == 2)
|
||||
case .carried: #expect(index == 3)
|
||||
case .partiallyDelivered: #expect(index == 4)
|
||||
case .delivered: #expect(index == 5)
|
||||
case .read: #expect(index == 6)
|
||||
case .notSentYet: #expect(index == 0)
|
||||
case .failed: #expect(index == 1)
|
||||
case .sending: #expect(index == 2)
|
||||
case .sent: #expect(index == 3)
|
||||
case .carried: #expect(index == 4)
|
||||
case .partiallyDelivered: #expect(index == 5)
|
||||
case .delivered: #expect(index == 6)
|
||||
case .read: #expect(index == 7)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -222,7 +222,7 @@ struct BLEFileTransferHandlerTests {
|
||||
#expect(message?.isPrivate == false)
|
||||
#expect(message?.senderPeerID == remotePeerID)
|
||||
#expect(message?.timestamp == Date(timeIntervalSince1970: 900))
|
||||
#expect(message?.deliveryStatus == nil)
|
||||
#expect(message?.deliveryStatus == .notSentYet)
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@ -29,7 +29,7 @@ public final class BitchatMessage: Codable {
|
||||
public let recipientNickname: String?
|
||||
public let senderPeerID: PeerID?
|
||||
public let mentions: [String]? // Array of mentioned nicknames
|
||||
public var deliveryStatus: DeliveryStatus? // Delivery tracking
|
||||
public var deliveryStatus: DeliveryStatus // Delivery tracking
|
||||
/// True when this message reached us across a mesh bridge (signed by its
|
||||
/// author for an internet rendezvous) rather than over local radio.
|
||||
public let isBridged: Bool
|
||||
@ -64,7 +64,9 @@ public final class BitchatMessage: Codable {
|
||||
recipientNickname = try container.decodeIfPresent(String.self, forKey: .recipientNickname)
|
||||
senderPeerID = try container.decodeIfPresent(PeerID.self, forKey: .senderPeerID)
|
||||
mentions = try container.decodeIfPresent([String].self, forKey: .mentions)
|
||||
deliveryStatus = try container.decodeIfPresent(DeliveryStatus.self, forKey: .deliveryStatus)
|
||||
// Archives written while the field was optional omit it for public
|
||||
// messages; absent means the message never entered a send pipeline.
|
||||
deliveryStatus = try container.decodeIfPresent(DeliveryStatus.self, forKey: .deliveryStatus) ?? .notSentYet
|
||||
// Absent in archives written before bridging existed.
|
||||
isBridged = try container.decodeIfPresent(Bool.self, forKey: .isBridged) ?? false
|
||||
}
|
||||
@ -93,7 +95,7 @@ public final class BitchatMessage: Codable {
|
||||
self.recipientNickname = recipientNickname
|
||||
self.senderPeerID = senderPeerID
|
||||
self.mentions = mentions
|
||||
self.deliveryStatus = deliveryStatus ?? (isPrivate ? .sending : nil)
|
||||
self.deliveryStatus = deliveryStatus ?? (isPrivate ? .sending : .notSentYet)
|
||||
self.isBridged = isBridged
|
||||
}
|
||||
}
|
||||
|
||||
@ -9,6 +9,7 @@
|
||||
import struct Foundation.Date
|
||||
|
||||
public enum DeliveryStatus: Codable, Equatable, Hashable {
|
||||
case notSentYet // Created but not yet handed to any transport
|
||||
case sending
|
||||
case sent // Left our device
|
||||
case carried // Sealed envelope handed to a courier; best-effort physical delivery
|
||||
@ -19,6 +20,8 @@ public enum DeliveryStatus: Codable, Equatable, Hashable {
|
||||
|
||||
public var displayText: String {
|
||||
switch self {
|
||||
case .notSentYet:
|
||||
return "Not sent yet"
|
||||
case .sending:
|
||||
return "Sending..."
|
||||
case .sent:
|
||||
|
||||
@ -0,0 +1,59 @@
|
||||
//
|
||||
// DeliveryStatusNotSentYetTests.swift
|
||||
// bitchatTests
|
||||
//
|
||||
// DeliveryStatus is a total state machine: every message carries a concrete
|
||||
// status from creation. Public messages start .notSentYet, private messages
|
||||
// keep their historical .sending default, and archives persisted while the
|
||||
// field was optional decode with the absent field mapped to .notSentYet.
|
||||
// This is free and unencumbered software released into the public domain.
|
||||
// For more information, see <https://unlicense.org>
|
||||
//
|
||||
|
||||
import Testing
|
||||
import Foundation
|
||||
@testable import BitFoundation
|
||||
|
||||
struct DeliveryStatusNotSentYetTests {
|
||||
|
||||
private func makeMessage(isPrivate: Bool, deliveryStatus: DeliveryStatus? = nil) -> BitchatMessage {
|
||||
BitchatMessage(
|
||||
sender: "alice",
|
||||
content: "hello",
|
||||
timestamp: Date(timeIntervalSince1970: 1_000),
|
||||
isRelay: false,
|
||||
isPrivate: isPrivate,
|
||||
deliveryStatus: deliveryStatus
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
func publicMessagesStartNotSentYetAndPrivateStartSending() {
|
||||
#expect(makeMessage(isPrivate: false).deliveryStatus == .notSentYet)
|
||||
#expect(makeMessage(isPrivate: true).deliveryStatus == .sending)
|
||||
// An explicit status always wins over the defaults.
|
||||
#expect(makeMessage(isPrivate: false, deliveryStatus: .sent).deliveryStatus == .sent)
|
||||
}
|
||||
|
||||
@Test
|
||||
func decodingLegacyArchiveWithoutStatusYieldsNotSentYet() throws {
|
||||
// Pre-existing archives omitted the key for public messages while the
|
||||
// field was optional; absent must map to .notSentYet, not fail.
|
||||
let encoded = try JSONEncoder().encode(makeMessage(isPrivate: false))
|
||||
var json = try #require(
|
||||
JSONSerialization.jsonObject(with: encoded) as? [String: Any]
|
||||
)
|
||||
json.removeValue(forKey: "deliveryStatus")
|
||||
let legacyData = try JSONSerialization.data(withJSONObject: json)
|
||||
|
||||
let decoded = try JSONDecoder().decode(BitchatMessage.self, from: legacyData)
|
||||
#expect(decoded.deliveryStatus == .notSentYet)
|
||||
}
|
||||
|
||||
@Test
|
||||
func decodingRoundTripPreservesConcreteStatus() throws {
|
||||
let message = makeMessage(isPrivate: true, deliveryStatus: .delivered(to: "bob", at: Date(timeIntervalSince1970: 2_000)))
|
||||
let decoded = try JSONDecoder().decode(BitchatMessage.self, from: JSONEncoder().encode(message))
|
||||
#expect(decoded.deliveryStatus == .delivered(to: "bob", at: Date(timeIntervalSince1970: 2_000)))
|
||||
}
|
||||
}
|
||||
Loading…
x
Reference in New Issue
Block a user