Skip to content

Commit adb60ed

Browse files
committed
feat(room-worker): actor-named, count-based membership system messages
Make create-channel / add-members / remove-member system messages actor-named and count-based so the frontend can render clickable lists of who was added/removed: - members_added (add + create): "Alice added 2 people and 1 organization to the chatroom", with a single shared addedContent helper used by both flows; a lone individual (no orgs) still renders the named form. Payload lists are the resolved member set — req.Users for add, req.ResolvedUsers for create (channel-expanded individuals included, org members represented as orgs). An empty channel emits no members_added (room_created still fires). - member_removed / member_left: actor-named removals ("Alice removed Bob from the chatroom"); wording "channel" -> "chatroom" across all membership messages. - Empty individuals/orgs lists marshal as [] (not null) for the frontend. - docs/client-api.md: document the members_added / member_removed / member_left sysMsgData payload schema. - history-service: integration test asserting sysMsgData survives the load-history round-trip. No pkg/model struct changes, no room-service/materialization changes, and no new database reads. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WuJMTQH2mnUqKd6jhSg8HK
1 parent b556c24 commit adb60ed

8 files changed

Lines changed: 380 additions & 77 deletions

File tree

docs/client-api.md

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2322,14 +2322,62 @@ Used by every history-service method that returns messages. Mirrors the Cassandr
23222322
| `reactions` | map<emoji, [ReactionUser](#reactionuser)[]> | Optional. Omitted when absent; `{}` when present but empty. |
23232323
| `deleted` | boolean | Optional. `true` for tombstoned messages. |
23242324
| `type` | string | Optional. System-message type when set; regular messages omit it. Known values: `"room_created"`, `"members_added"`, `"member_removed"`, `"member_left"`, `"room_renamed"`, `"room_restricted"`. For all six, `msg` is populated with a server-rendered human-readable body and `sender.account` is the responsible actor (the requester for adds/removes-by-other / room-creates / renames / restricted changes, the leaving user for self-leave). |
2325-
| `sysMsgData` | string | Optional. Base64-encoded raw JSON payload for system messages. |
2325+
| `sysMsgData` | string | Optional. Base64-encoded JSON payload for system messages; shape depends on `type` (see [System-message `sysMsgData` payloads](#system-message-sysmsgdata-payloads)). |
23262326
| `siteId` | string | Optional. The site that owns the message. |
23272327
| `editedAt` | string | Optional. RFC 3339. Set after an edit. |
23282328
| `updatedAt` | string | Optional. RFC 3339. Mirrors `editedAt` for edits, set on delete to record the deletion time. |
23292329
| `threadRoomId` | string | Optional. The thread room ID when this is a thread message. |
23302330
| `pinnedAt` | string | Optional. RFC 3339. With the `messages_by_room` `pinned_at` mirror, room-timeline history loads now return this on pinned rows too (previously only `pin.list` and point lookups carried it). |
23312331
| `pinnedBy` | [MessageParticipant](#messageparticipant) | Optional. |
23322332

2333+
##### System-message `sysMsgData` payloads
2334+
2335+
`sysMsgData` is base64-encoded JSON whose shape depends on `type`.
2336+
2337+
`members_added` (also emitted on room creation):
2338+
2339+
| field | type | description |
2340+
|-------|------|-------------|
2341+
| `individuals` | string[] | Accounts of the individuals added (direct + channel-expanded; excludes organization members). Empty `[]`, never `null`. The "n people" count is `individuals.length`. |
2342+
| `orgs` | string[] | Organization IDs added. Empty `[]`, never `null`. The "m organizations" count is `orgs.length`. |
2343+
| `channels` | [ChannelRef](#channelref)[] | Source channels whose members were copied in (provenance; may be empty). |
2344+
| `addedUsersCount` | number | Total members materialized by the operation, including organization-expanded members; may differ from `individuals.length`. |
2345+
2346+
```json
2347+
{ "individuals": ["alice", "bob"], "orgs": ["eng"], "channels": [], "addedUsersCount": 12 }
2348+
```
2349+
2350+
`member_removed`:
2351+
2352+
| field | type | description |
2353+
|-------|------|-------------|
2354+
| `user` | [SysMsgUser](#sysmsguser) | Set when an individual was removed. |
2355+
| `orgId` | string | Set when an organization was removed. |
2356+
| `sectName` | string | Display name of the removed organization (set with `orgId`). |
2357+
| `removedUsersCount` | number | Number of underlying accounts whose subscription was removed. |
2358+
2359+
```json
2360+
{ "user": { "account": "bob", "engName": "Bob", "chineseName": "鮑勃" }, "removedUsersCount": 1 }
2361+
```
2362+
2363+
`member_left``{ "user": SysMsgUser }` for the user who left:
2364+
2365+
```json
2366+
{ "user": { "account": "bob", "engName": "Bob", "chineseName": "鮑勃" } }
2367+
```
2368+
2369+
The displayed `msg` for `members_added` shows counts matching `individuals.length` / `orgs.length`, which clients may render as clickable lists.
2370+
2371+
##### SysMsgUser
2372+
2373+
A user referenced by a system-message payload.
2374+
2375+
| field | type | description |
2376+
|-------|------|-------------|
2377+
| `account` | string | The user's account. |
2378+
| `engName` | string | English display name; may be empty. |
2379+
| `chineseName` | string | Chinese display name; may be empty. |
2380+
23332381
##### MessageParticipant
23342382

23352383
The author/mention/pinner embedded in a message. Distinct from the event-actor
@@ -4683,7 +4731,7 @@ The canonical broadcast message (distinct from the history [Message schema](#mes
46834731
| `threadParentMessageCreatedAt` | string | Optional. RFC 3339. |
46844732
| `tshow` | boolean | Optional. Whether a thread reply is also shown in the parent room. |
46854733
| `type` | string | Optional. System-message type when set. |
4686-
| `sysMsgData` | string | Optional. Base64-encoded raw JSON payload for system messages. |
4734+
| `sysMsgData` | string | Optional. Base64-encoded JSON payload for system messages; shape depends on `type` (see [System-message `sysMsgData` payloads](#system-message-sysmsgdata-payloads)). |
46874735
| `quotedParentMessage` | [QuotedParentMessage](#quotedparentmessage) | Optional. |
46884736
| `pinnedAt` | string | Optional. RFC 3339. |
46894737
| `pinnedBy` | [Participant](#participant) | Optional. |

history-service/internal/cassrepo/messages_by_room_integration_test.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ package cassrepo
44

55
import (
66
"context"
7+
"encoding/json"
78
"fmt"
89
"testing"
910
"time"
@@ -15,6 +16,7 @@ import (
1516

1617
"github.com/hmchangw/chat/history-service/internal/models"
1718
"github.com/hmchangw/chat/pkg/atrest"
19+
"github.com/hmchangw/chat/pkg/model"
1820
cassmodel "github.com/hmchangw/chat/pkg/model/cassandra"
1921
"github.com/hmchangw/chat/pkg/msgbucket"
2022
"github.com/hmchangw/chat/pkg/testutil"
@@ -335,3 +337,71 @@ func TestGetMessages_DecryptErrorHaltsWalk(t *testing.T) {
335337
assert.ErrorIs(t, walkErr, atrest.ErrPayloadMalformed,
336338
"walk must halt on the day-1 malformed-nonce error; surfacing ErrAuthFailed means the walk continued past day1 and let day2 overwrite scanErr")
337339
}
340+
341+
func TestGetMessagesBefore_SysMsgDataRoundTrips(t *testing.T) {
342+
session := setupCassandra(t)
343+
sizer := msgbucket.New(24 * time.Hour)
344+
repo := NewRepository(session, sizer, 365, nil)
345+
ctx := context.Background()
346+
347+
roomID := "room-sysmsg"
348+
createdAt := time.Now().UTC().Truncate(time.Millisecond)
349+
sender := models.Participant{ID: "u_a", Account: "alice"}
350+
351+
payload, err := json.Marshal(model.MembersAdded{Individuals: []string{"u1", "u2"}, Orgs: []string{"o1"}, AddedUsersCount: 3})
352+
require.NoError(t, err)
353+
354+
require.NoError(t, session.Query(
355+
`INSERT INTO messages_by_room (room_id, bucket, created_at, message_id, sender, msg, type, sys_msg_data) VALUES (?, ?, ?, ?, ?, ?, ?, ?)`,
356+
roomID, sizer.Of(createdAt), createdAt, "m-sys", sender,
357+
`"alice" added 2 people and 1 organization to the chatroom`,
358+
model.MessageTypeMembersAdded, payload,
359+
).Exec())
360+
361+
page, err := repo.GetMessagesBefore(ctx, roomID, createdAt.Add(time.Second), time.Time{}, PageRequest{PageSize: 10})
362+
require.NoError(t, err)
363+
require.Len(t, page.Data, 1)
364+
365+
got := page.Data[0]
366+
assert.Equal(t, model.MessageTypeMembersAdded, got.Type)
367+
require.NotEmpty(t, got.SysMsgData, "sysMsgData must survive the load-history round-trip")
368+
369+
var decoded model.MembersAdded
370+
require.NoError(t, json.Unmarshal(got.SysMsgData, &decoded))
371+
assert.Equal(t, []string{"u1", "u2"}, decoded.Individuals)
372+
assert.Equal(t, []string{"o1"}, decoded.Orgs)
373+
assert.Equal(t, 3, decoded.AddedUsersCount, "whole payload (not just the slices) must survive")
374+
}
375+
376+
// An orgs-only add stores Individuals as []; it must survive as [] (non-nil),
377+
// not null, so the frontend can read .length without a guard.
378+
func TestGetMessagesBefore_SysMsgDataEmptySlice(t *testing.T) {
379+
session := setupCassandra(t)
380+
sizer := msgbucket.New(24 * time.Hour)
381+
repo := NewRepository(session, sizer, 365, nil)
382+
ctx := context.Background()
383+
384+
roomID := "room-sysmsg-empty"
385+
createdAt := time.Now().UTC().Truncate(time.Millisecond)
386+
sender := models.Participant{ID: "u_a", Account: "alice"}
387+
388+
payload, err := json.Marshal(model.MembersAdded{Individuals: []string{}, Orgs: []string{"o1"}})
389+
require.NoError(t, err)
390+
391+
require.NoError(t, session.Query(
392+
`INSERT INTO messages_by_room (room_id, bucket, created_at, message_id, sender, msg, type, sys_msg_data) VALUES (?, ?, ?, ?, ?, ?, ?, ?)`,
393+
roomID, sizer.Of(createdAt), createdAt, "m-sys-empty", sender,
394+
`"alice" added 1 organization to the chatroom`,
395+
model.MessageTypeMembersAdded, payload,
396+
).Exec())
397+
398+
page, err := repo.GetMessagesBefore(ctx, roomID, createdAt.Add(time.Second), time.Time{}, PageRequest{PageSize: 10})
399+
require.NoError(t, err)
400+
require.Len(t, page.Data, 1)
401+
402+
var decoded model.MembersAdded
403+
require.NoError(t, json.Unmarshal(page.Data[0].SysMsgData, &decoded))
404+
require.NotNil(t, decoded.Individuals, "empty individuals must survive as [] not null")
405+
assert.Empty(t, decoded.Individuals)
406+
assert.Equal(t, []string{"o1"}, decoded.Orgs)
407+
}

pkg/model/member.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,9 @@ type MentionableSubscriptionsResponse struct {
206206

207207
// CreateRoomRequest is the canonical event payload (X-Request-ID rides on the NATS header).
208208
// Users/Orgs/Channels are the literal client request; ResolvedUsers/ResolvedOrgs carry the
209-
// post-expansion (channel-ref-merged, requester-stripped, dedup'd) sets the worker uses for
210-
// member materialization. Sys-message payloads use the literal lists.
209+
// post-expansion (channel-ref-merged, requester-stripped, dedup'd) sets. The worker uses the
210+
// resolved sets for member materialization and the members_added sys-message; room_created
211+
// uses the literal lists.
211212
type CreateRoomRequest struct {
212213
Name string `json:"name" bson:"name"`
213214
Users []string `json:"users" bson:"users"`

room-worker/handler.go

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ func (h *Handler) processRemoveIndividual(ctx context.Context, req *model.Remove
449449
if isSelfLeave {
450450
content = formatLeft(&user.User)
451451
} else {
452-
content = formatRemovedUser(&user.User)
452+
content = formatRemovedUser(requester, &user.User)
453453
}
454454
sysMsg := model.Message{
455455
ID: idgen.MessageIDFromRequestID(seed, "rmindiv"),
@@ -652,7 +652,7 @@ func (h *Handler) processRemoveOrg(ctx context.Context, req *model.RemoveMemberR
652652
UserID: requester.ID,
653653
UserAccount: requester.Account,
654654
Type: model.MessageTypeMemberRemoved,
655-
Content: formatRemovedOrg(name, tcName, req.OrgID),
655+
Content: formatRemovedOrg(requester, name, tcName, req.OrgID),
656656
SysMsgData: sysMsgPayload,
657657
CreatedAt: now,
658658
}
@@ -1042,8 +1042,8 @@ func (h *Handler) processAddMembers(ctx context.Context, data []byte) (err error
10421042
// (actualAccounts) or new org rows (req.Orgs). The org→individual upgrade
10431043
// path (only needIRM populated) writes the missing individual room_members
10441044
// row silently — no membership state changed for the room itself, so
1045-
// emitting an empty MemberAddEvent and a "added members to the channel"
1046-
// sys-msg with no actual members listed would mislead end users.
1045+
// emitting an empty MemberAddEvent and a members_added sys-msg with no
1046+
// actual members listed would mislead end users.
10471047
historySharedSince := historySharedSincePtr(req.History, req.Timestamp, req.RoomID)
10481048
if len(actualAccounts) > 0 || len(req.Orgs) > 0 {
10491049
memberAddEvt := model.MemberAddEvent{
@@ -1086,21 +1086,23 @@ func (h *Handler) processAddMembers(ctx context.Context, data []byte) (err error
10861086
}
10871087
}
10881088

1089+
// Individuals = req.Users (direct + channel individuals, already merged
1090+
// by room-service; org members ride in Orgs). Counts are list lengths.
10891091
membersAdded := model.MembersAdded{
1090-
Individuals: actualAccounts,
1091-
Orgs: req.Orgs,
1092+
Individuals: nonNil(req.Users),
1093+
Orgs: nonNil(req.Orgs),
10921094
Channels: req.Channels,
10931095
AddedUsersCount: len(subs),
10941096
}
10951097
sysMsgData, _ := json.Marshal(membersAdded)
10961098
seed := messageDedupSeed(ctx, "processAddMembers", req.RoomID,
10971099
fmt.Sprintf("%s:%s:%d", req.RoomID, req.RequesterAccount, req.Timestamp))
1098-
// Single form only for direct 1-user adds; org-bearing adds always use multi.
1099-
content := formatAddedMulti(requester)
1100-
if len(subs) == 1 && len(req.Orgs) == 0 {
1101-
onlyUser := userMap[subs[0].User.Account]
1102-
content = formatAddedSingle(requester, &onlyUser)
1103-
}
1100+
content := addedContent(requester, req.Users, req.Orgs, func(a string) *model.User {
1101+
if u, ok := userMap[a]; ok {
1102+
return &u
1103+
}
1104+
return nil
1105+
})
11041106
sysMsg := model.Message{
11051107
ID: idgen.MessageIDFromRequestID(seed, "addmembers"),
11061108
RoomID: req.RoomID,
@@ -1503,7 +1505,7 @@ func (h *Handler) finishCreateRoom(ctx context.Context, req *model.CreateRoomReq
15031505

15041506
// Task 36: channel-only sys-messages
15051507
if room.Type == model.RoomTypeChannel {
1506-
if err := h.publishChannelSysMessages(ctx, req, room, requester, len(subs)-1, requestID, now); err != nil {
1508+
if err := h.publishChannelSysMessages(ctx, req, room, requester, userByAccount, len(subs)-1, requestID, now); err != nil {
15071509
return fmt.Errorf("publish sys messages: %w", err)
15081510
}
15091511
}
@@ -1589,7 +1591,7 @@ func (h *Handler) finishCreateRoom(ctx context.Context, req *model.CreateRoomReq
15891591
return nil
15901592
}
15911593

1592-
func (h *Handler) publishChannelSysMessages(ctx context.Context, req *model.CreateRoomRequest, room *model.Room, requester *model.User, addedUsersCount int, requestID string, now time.Time) error {
1594+
func (h *Handler) publishChannelSysMessages(ctx context.Context, req *model.CreateRoomRequest, room *model.Room, requester *model.User, userByAccount map[string]*model.User, addedUsersCount int, requestID string, now time.Time) error {
15931595
acceptedAt := time.UnixMilli(req.Timestamp).UTC()
15941596

15951597
sysData1, err := json.Marshal(model.RoomCreated{
@@ -1616,22 +1618,32 @@ func (h *Handler) publishChannelSysMessages(ctx context.Context, req *model.Crea
16161618
return fmt.Errorf("publish room_created: %w", err)
16171619
}
16181620

1621+
// ResolvedUsers = resolved individuals, already creator-stripped and
1622+
// org-member-free by room-service; Orgs counted as orgs, not expanded.
1623+
if len(req.ResolvedUsers) == 0 && len(req.ResolvedOrgs) == 0 {
1624+
// Nothing added beyond the creator (e.g. an empty channel); room_created
1625+
// already fired, so do not emit a degenerate "added 0 people" message.
1626+
return nil
1627+
}
16191628
sysData2, err := json.Marshal(model.MembersAdded{
1620-
Individuals: req.Users,
1621-
Orgs: req.Orgs,
1629+
Individuals: nonNil(req.ResolvedUsers),
1630+
Orgs: nonNil(req.ResolvedOrgs),
16221631
Channels: req.Channels,
16231632
AddedUsersCount: addedUsersCount,
16241633
})
16251634
if err != nil {
16261635
return fmt.Errorf("marshal members_added sys data: %w", err)
16271636
}
1637+
content := addedContent(requester, req.ResolvedUsers, req.ResolvedOrgs, func(a string) *model.User {
1638+
return userByAccount[a]
1639+
})
16281640
msg2 := model.Message{
16291641
ID: idgen.MessageIDFromRequestID(requestID, "members_added"),
16301642
RoomID: room.ID,
16311643
UserID: requester.ID,
16321644
UserAccount: requester.Account,
16331645
Type: model.MessageTypeMembersAdded,
1634-
Content: formatAddedMulti(requester),
1646+
Content: content,
16351647
SysMsgData: sysData2,
16361648
CreatedAt: acceptedAt.Add(time.Millisecond),
16371649
}

0 commit comments

Comments
 (0)