Skip to content

chat: failing test - the pending message activity names no account - #29835

Open
chrisnojima wants to merge 1 commit into
masterfrom
nojima/HOTPOT-outbox-uid-clean
Open

chrisnojima wants to merge 1 commit into
masterfrom
nojima/HOTPOT-outbox-uid-clean

Conversation

@chrisnojima

Copy link
Copy Markdown
Contributor

Summary

A failing test only, for review on its own: the pending (outbox) message activity goes out without an account id.

PostTextNonblock
  build MessagePlaintext            # ClientHeader.Sender not set yet
  NonblockingSender.Send
    outbox.PushMessage
      newMessageNotifier(obr)       # chat/deliverer.go
        uid := obr.Msg.ClientHeader.Sender    # empty
        ActivityNotifier.Activity(ctx, uid, ...)  -> NewChatActivity{uid: ""}

Since #29804 the app drops chat notifications that don't name the signed-in account. So on desktop it never shows a message as pending: no pending row, no sending indicator, and no deleting indicator on a message just sent. The outbox notification arrives (with the outbox ordinal), and is discarded.

TestChatSrvPendingMessageActivityNamesTheAccount posts through PostTextNonblock and asserts the outbox activity names the poster. The likely fix is one line in chat/deliverer.go: the outbox is per account (Deliverer.Start(ctx, uid)), so the notifier can use that uid instead of the record's header.

Evidence

Observed in the desktop app (renderer RPC log) for one send, uid as received:

NewChatActivity  outbox  ordinal 3896.1  outboxID d424…  uid ""
NewChatActivity  valid   messageID 3897  outboxID d424…  uid <account>

The new test compiles and is clean under go vet and golangci-lint --new-from-rev master. It needs kbweb (fake-user signup), which wasn't available where it was written, so it hasn't run locally: CI is where it fails.

Merge Danger

Door: two-way

Blast Radius: none

A test file only.

PostTextNonblock queues the message in the outbox before its client header has a sender, and the
deliverer's outbox notifier takes the activity's uid from that header, so the pending (outbox)
incoming-message activity goes out with an empty uid. The app drops chat notifications that do
not name the signed-in account (#29804), so it never shows a pending message: no pending row, no
sending indicator, and no deleting indicator on a message just sent.

This test posts through PostTextNonblock and asserts the outbox activity names the poster.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants