Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions docs/client-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -801,9 +801,10 @@ Shared by Add Members, Remove Member, and Update Member Role.
| `userId` | string | The affected user's internal user ID. Omitted on the org-removal path (only `subscription.u.account` is set there). |
| `subscription` | [Subscription](#subscription) | For `added` / `role_updated`: the full Subscription record. For `removed`: a [RemovedSubscriptionRef](#removedsubscriptionref) lean ref (see Remove Member). |
| `action` | string | `"added"`, `"removed"`, `"role_updated"`, `"mute_toggled"`, or `"favorite_toggled"`. |
| `roomName` | string | Per-subscriber display label, set only where the server already has the name. On `added`: `channel` → room name; `dm` → counterpart's display name (`engName` + `chineseName`, falling back to account); `botDM` → the bot's app name. On `role_updated`: the channel name. Omitted (`omitempty`) on `mute_toggled` / `favorite_toggled` / `read`, and absent on `removed`. |
| `timestamp` | number | Epoch ms (UTC). |

On `added` / `role_updated` / `mute_toggled` / `favorite_toggled` the embedded `Subscription` serializes its ID as `id` (not `_id`) and the user under `u` (not `user`). Non-`omitempty` fields (`id`, `u`, `roomId`, `siteId`, `roles`, `name`, `roomType`, `joinedAt`, `hasMention`, `alert`, `muted`, `favorite`) are always present. `removed` events use a dedicated lean payload (`SubscriptionRemovedEvent`) whose `subscription` carries **only** `roomId`, `roomType`, and `u` — no zero-valued `Subscription` fields are sent.
On `added` / `role_updated` / `mute_toggled` / `favorite_toggled` the embedded `Subscription` serializes its ID as `id` (not `_id`) and the user under `u` (not `user`). Non-`omitempty` fields (`id`, `u`, `roomId`, `siteId`, `roles`, `name`, `roomType`, `joinedAt`, `hasMention`, `alert`, `muted`, `favorite`) are always present — and the envelope's `roomName` is always present as a field (empty on `mute_toggled` / `favorite_toggled`). `removed` events use a dedicated lean payload (`SubscriptionRemovedEvent`) whose `subscription` carries **only** `roomId`, `roomType`, and `u` — no zero-valued `Subscription` fields are sent.

```json
{
Expand All @@ -818,6 +819,7 @@ On `added` / `role_updated` / `mute_toggled` / `favorite_toggled` the embedded `
"joinedAt": "2026-05-06T08:01:23Z"
},
"action": "added",
"roomName": "engineering-announcements",
"timestamp": 1746518483000
}
```
Expand Down Expand Up @@ -1015,6 +1017,7 @@ See [Error envelope](#6-error-envelope-reference). Returned synchronously when v
"joinedAt": "2026-05-06T08:01:23Z"
},
"action": "role_updated",
"roomName": "engineering-announcements",
"timestamp": 1746518483000
}
```
Expand Down Expand Up @@ -1239,7 +1242,7 @@ See [Error envelope](#6-error-envelope-reference). Common errors: `"only room me

| Field | Type | Required | Notes |
|---|---|---|---|
| `limit` | number | no | When omitted, the server uses `min(3, room.userCount)` (so a 2-member room returns 2 rows, an empty room returns an empty list). When supplied, must be `> 0` and `<= room.userCount`. |
| `limit` | number | no | Upper bound on returned rows. When omitted, the server uses `min(3, room.userCount)` (an empty room returns an empty list); when supplied, must be `> 0` and `<= room.userCount`. Fewer rows may come back — members with an empty `statusText` are omitted (see `members`). |

```json
{ "limit": 5 }
Expand All @@ -1249,7 +1252,7 @@ See [Error envelope](#6-error-envelope-reference). Common errors: `"only room me

| Field | Type | Notes |
|---|---|---|
| `members` | array<MemberStatus> | One entry per room subscription, projected from the joined `users` document. |
| `members` | array<MemberStatus> | One entry per room subscription **with a non-empty `statusText`**, projected from the joined `users` document. Members without a status set are omitted. |

`MemberStatus`:

Expand All @@ -1259,7 +1262,7 @@ See [Error envelope](#6-error-envelope-reference). Common errors: `"only room me
| `engName` | string | English display name. |
| `chineseName` | string | Chinese display name. |
| `statusIsShow` | boolean | Whether the user has chosen to surface their status text. |
| `statusText` | string | Free-form presence text (e.g. `"available"`, `"in a meeting"`). Empty for users who have never set a status. |
| `statusText` | string | Free-form presence text (e.g. `"available"`, `"in a meeting"`); always non-empty — members without a status are omitted from `members`. |
Comment thread
vjauhari-work marked this conversation as resolved.

```json
{
Expand Down
13 changes: 0 additions & 13 deletions pkg/model/account.go

This file was deleted.

34 changes: 0 additions & 34 deletions pkg/model/account_test.go

This file was deleted.

1 change: 1 addition & 0 deletions pkg/model/event.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ type SubscriptionUpdateEvent struct {
UserID string `json:"userId"`
Subscription Subscription `json:"subscription"`
Action string `json:"action"` // "added" | "removed" | "role_updated" | "mute_toggled" | "favorite_toggled" | "read"
RoomName string `json:"roomName,omitempty"`
Timestamp int64 `json:"timestamp" bson:"timestamp"`
}

Expand Down
67 changes: 67 additions & 0 deletions pkg/model/model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ func TestIsPlatformAdmin(t *testing.T) {
{"admin role present", &model.User{Account: "alice", Roles: []model.UserRole{model.UserRoleAdmin}}, true},
{"admin among many", &model.User{Account: "alice", Roles: []model.UserRole{model.UserRoleUser, model.UserRoleAdmin, "auditor"}}, true},
{"case-sensitive (Admin not admin)", &model.User{Account: "alice", Roles: []model.UserRole{"Admin"}}, false},
{"p_ account without admin role is not a role admin", &model.User{Account: "p_webhook"}, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand All @@ -70,6 +71,49 @@ func TestIsPlatformAdmin(t *testing.T) {
}
}

func TestIsPlatformAdminAccount(t *testing.T) {
tests := []struct {
name string
account string
want bool
}{
{"p_ prefix webhook", "p_webhook", true},
{"bare p_ prefix", "p_", true},
{"plain human account", "alice", false},
{"bot account", "weather.bot", false},
{"case-sensitive prefix (P_)", "P_upper", false},
{"p underscore not at start", "alice_p_x", false},
{"empty", "", false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, model.IsPlatformAdminAccount(tt.account))
})
}
}

func TestIsBot(t *testing.T) {
tests := []struct {
name string
account string
want bool
}{
{"dot-bot suffix", "weather.bot", true},
{"bare dot-bot", ".bot", true},
{"p_ prefix is not a bot", "p_webhook", false},
{"plain human account", "alice", false},
{"ends with bot but no dot", "robot", false},
{"dot-bot not at end", "alice.bot.com", false},
{"case-sensitive suffix", "weather.BOT", false},
{"empty", "", false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, model.IsBot(tt.account))
})
}
}

func TestUserJSON_WithStatus(t *testing.T) {
u := model.User{
ID: "u1", Account: "alice", SiteID: "site-a",
Expand Down Expand Up @@ -3892,3 +3936,26 @@ func TestSubscriptionBaseMetadata_RoundTrip(t *testing.T) {
assert.False(t, present, "%q must be omitted when unset", k)
}
}

func TestSubscriptionUpdateEvent_RoomNameRoundTrips(t *testing.T) {
evt := model.SubscriptionUpdateEvent{
UserID: "u1",
Subscription: model.Subscription{ID: "s1", RoomID: "r1", RoomType: model.RoomTypeDM},
Action: "added",
RoomName: "Alice Wang",
Timestamp: 1735689600000,
}
data, err := json.Marshal(&evt)
require.NoError(t, err)
assert.Contains(t, string(data), `"roomName":"Alice Wang"`)

var dst model.SubscriptionUpdateEvent
require.NoError(t, json.Unmarshal(data, &dst))
assert.Equal(t, "Alice Wang", dst.RoomName)
}

func TestSubscriptionUpdateEvent_RoomNameOmittedWhenEmpty(t *testing.T) {
data, err := json.Marshal(&model.SubscriptionUpdateEvent{Action: "mute_toggled"})
require.NoError(t, err)
assert.NotContains(t, string(data), "roomName")
}
16 changes: 14 additions & 2 deletions pkg/model/user.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package model

import "strings"

// UserRole is a platform-level role flag on the User record.
// Empty Roles reads as ["user"]; only positive marker is "admin".
type UserRole string
Expand Down Expand Up @@ -27,8 +29,7 @@ type User struct {
Roles []UserRole `json:"roles,omitempty" bson:"roles,omitempty"`
}

// IsPlatformAdmin reports whether u holds the platform admin role.
// Returns false for nil receivers and for users without the role.
// IsPlatformAdmin reports whether u holds the platform admin role. Nil-safe.
func IsPlatformAdmin(u *User) bool {
if u == nil {
return false
Expand All @@ -41,6 +42,17 @@ func IsPlatformAdmin(u *User) bool {
return false
}

// IsPlatformAdminAccount reports whether account is a platform-admin account
// (a "p_" prefix).
func IsPlatformAdminAccount(account string) bool {
return strings.HasPrefix(account, "p_")
}

// IsBot reports whether account is a bot account (a ".bot" suffix).
func IsBot(account string) bool {
return strings.HasSuffix(account, ".bot")
}

// DisplayName renders the user's display label for Drive ownership metadata:
// the account when either name is missing, the English name when both names are
// identical, otherwise "<engName> <chineseName>".
Expand Down
35 changes: 19 additions & 16 deletions room-service/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ func classifyAndValidate(req *model.CreateRoomRequest, requesterAccount string)
return "", errChannelNameTooLong
}
for _, a := range req.Users {
if isBot(a) {
if model.IsBot(a) || model.IsPlatformAdminAccount(a) {
return "", errBotInChannel
}
}
Expand Down Expand Up @@ -736,7 +736,8 @@ func (h *Handler) updateRole(c *natsrouter.Context, req model.UpdateRoleRequest)
return nil, fmt.Errorf("set owner role: %w", err)
}

subEvtData, err := h.publishSubscriptionUpdate(ctx, req.Account, "role_updated", sub, now)
// Role updates are channel-only (guarded above); the channel name is already in hand.
subEvtData, err := h.publishSubscriptionUpdate(ctx, req.Account, "role_updated", sub, room.Name, now)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -765,24 +766,23 @@ func (h *Handler) updateRole(c *natsrouter.Context, req model.UpdateRoleRequest)
return &model.StatusReply{Status: "ok"}, nil
}

// publishSubscriptionUpdate marshals a SubscriptionUpdateEvent for sub with the
// given action and best-effort publishes it to the account's subscription.update
// subject over core NATS. A publish failure is logged, not returned — the DB
// write is the source of truth; clients reconcile on next refetch. Returns the
// marshaled event so callers can reuse it (e.g. as a cross-site outbox payload).
func (h *Handler) publishSubscriptionUpdate(ctx context.Context, account, action string, sub *model.Subscription, ts time.Time) ([]byte, error) {
// publishSubscriptionUpdate best-effort publishes a SubscriptionUpdateEvent (sub, action,
// roomName) over core NATS; a publish failure is logged, not returned. Returns the marshaled event.
func (h *Handler) publishSubscriptionUpdate(ctx context.Context, account, action string, sub *model.Subscription, roomName string, ts time.Time) ([]byte, error) {
subEvt := model.SubscriptionUpdateEvent{
UserID: sub.User.ID,
Subscription: *sub,
Action: action,
RoomName: roomName,
Timestamp: ts.UnixMilli(),
}
data, err := json.Marshal(subEvt)
if err != nil {
return nil, fmt.Errorf("marshal subscription update event: %w", err)
}
if err := h.publishCore(ctx, subject.SubscriptionUpdate(account), data); err != nil {
slog.Error("subscription update publish failed", "error", err, "account", account)
slog.ErrorContext(ctx, "subscription update publish failed",
"request_id", natsutil.RequestIDFromContext(ctx), "error", err, "account", account)
}
return data, nil
}
Expand Down Expand Up @@ -825,7 +825,7 @@ func (h *Handler) addMembers(c *natsrouter.Context, req model.AddMembersRequest)
// create-channel: a client that explicitly lists a bot must see a hard
// error rather than a silent drop.
for _, a := range req.Users {
if isBot(a) {
if model.IsBot(a) || model.IsPlatformAdminAccount(a) {
return nil, errBotInChannel
}
}
Expand Down Expand Up @@ -1294,11 +1294,12 @@ func (h *Handler) messageRead(c *natsrouter.Context) (*model.StatusReply, error)
}

// Best-effort subscription.update to the reader's account (multi-device sync).
if !isBot(account) {
if !model.IsBot(account) {
updatedSub := *sub
updatedSub.LastSeenAt = &now
updatedSub.Alert = newAlert
if _, err := h.publishSubscriptionUpdate(ctx, account, "read", &updatedSub, now); err != nil {
// roomName omitted: clients don't use it on read events.
if _, err := h.publishSubscriptionUpdate(ctx, account, "read", &updatedSub, "", now); err != nil {
slog.Error("subscription update on read failed", "error", err,
"request_id", natsutil.RequestIDFromContext(ctx), "account", account)
}
Expand Down Expand Up @@ -1684,7 +1685,7 @@ func (h *Handler) roomRename(c *natsrouter.Context, req model.RoomRenameRequest)
return nil, errRenameChannelOnly
}

if !isPlatformAdmin(requesterUser) {
if !model.IsPlatformAdmin(requesterUser) {
sub, subErr := h.store.GetSubscription(ctx, account, roomID)
if subErr != nil {
if errors.Is(subErr, mongo.ErrNoDocuments) || errors.Is(subErr, model.ErrSubscriptionNotFound) {
Expand Down Expand Up @@ -1735,7 +1736,7 @@ func (h *Handler) roomRestricted(c *natsrouter.Context, req model.RoomRestricted
if getUserErr != nil && !errors.Is(getUserErr, ErrUserNotFound) {
return nil, fmt.Errorf("get user: %w", getUserErr)
}
if !isPlatformAdmin(requesterUser) {
if !model.IsPlatformAdmin(requesterUser) {
return nil, errOnlyAdmins
}

Expand Down Expand Up @@ -1894,7 +1895,8 @@ func (h *Handler) muteToggle(c *natsrouter.Context) (*model.MuteToggleResponse,
return nil, fmt.Errorf("toggle subscription mute: %w", err)
}

if _, err := h.publishSubscriptionUpdate(ctx, account, "mute_toggled", sub, now); err != nil {
// roomName omitted: the frontend doesn't use it for mute, so we avoid an extra lookup.
if _, err := h.publishSubscriptionUpdate(ctx, account, "mute_toggled", sub, "", now); err != nil {
return nil, err
}

Expand Down Expand Up @@ -1971,7 +1973,8 @@ func (h *Handler) favoriteToggle(c *natsrouter.Context) (*model.FavoriteToggleRe
return nil, fmt.Errorf("toggle subscription favorite: %w", err)
}

if _, err := h.publishSubscriptionUpdate(ctx, account, "favorite_toggled", sub, now); err != nil {
// roomName omitted: the frontend doesn't use it for favorite, so we avoid an extra lookup.
if _, err := h.publishSubscriptionUpdate(ctx, account, "favorite_toggled", sub, "", now); err != nil {
return nil, err
}

Expand Down
Loading
Loading