codereview修复:资料弹窗好友关系判断、转发内容克隆、账号切换缓存残留等

三路独立codereview(群成员UI+资料弹窗 / 未读红点+转发修复 / 搜索+复制修复)
发现并修复:

- UserCardRoot 资料弹窗此前完全没读 profile.relationship,对陌生人也直接
  显示"发消息"/通话按钮——现在按好友/待验证/对方已申请/不可添加/添加好友
  五态分支,和 FriendProfileView 保持一致,接入 AddFriendModal。
- useEscapeKey 注册位置错误:挂在常驻的 UserCardRoot 而不是随卡片开关的
  UserCardPanel,导致从其他弹层里打开资料卡片时,按 Escape 关掉的是底下
  那层弹层而不是卡片本身。
- "移出"格子的可见性此前只判断 isAdmin,在"群主+多个管理员、没有普通成员"
  的群里会显示格子但点进去一个能移出的都没有——改成同时判断成员列表里
  是否真的存在能移出的对象。
- flatForwardPayload 对图片/文件/语音/视频/名片/通话结果类消息,转发时直接
  返回了原消息的 content 对象引用而不是拷贝——SDK 出站前会就地删除假值
  字段,等于转发的同时悄悄改写了原消息在本地缓存里的内容。改成每次转发都
  返回浅拷贝;顺手让检查顺序和其余识别这几种格式的地方保持一致(先判类型
  明确的附件/名片/通话结果,再判无类型标记的纯文本),并补上和
  mobileAttachmentContentOf/contactCardContentOf 同样的字段合法性校验。
- confirmedReadSeq 等聊天列表相关的模块级缓存此前从未在登出/切换企业时清
  空——同一个标签页不刷新页面直接换账号登录,会把上一个账号的"已读到哪"
  当成新账号的下限,把新账号真实未读的自聊消息误判为已读。补了统一的
  reset监听机制(connection.ts 的 resetTinode 现在会广播),topics.ts 据此
  清空所有相关缓存。
- 复制功能补final `?? ''`兜底,防止附件数据本身不完整时仍复制出"undefined"。

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Your Name
2026-09-25 00:45:34 +08:00
co-authored by Claude Sonnet 5
parent 105ff76959
commit 0ba5c97d78
8 changed files with 204 additions and 59 deletions
+64 -3
View File
@@ -1,8 +1,11 @@
import { useState } from 'react';
import { useQuery } from '@tanstack/react-query';
import { useNavigate } from 'react-router-dom';
import { Avatar } from '@/app/Avatar';
import { Icon } from '@/app/Icon';
import { AddFriendModal } from '@/features/contacts/AddFriendModal';
import { ImApiError } from '@/lib/api/client';
import { getUserProfile } from '@/lib/api/friends';
import { useEscapeKey } from '@/lib/hooks';
@@ -18,9 +21,6 @@ const GENDER_LABEL: Record<string, string> = { male: '男', female: '女' };
* replacement for it -- "查看完整资料" below still routes there for remark-editing/QR code/etc. */
export function UserCardRoot() {
const userId = useUserCardStore((s) => s.userId);
// Always registered (hooks can't be conditional), matching ConfirmDialogRoot's own reasoning --
// a no-op onEscape while no card is open is harmless.
useEscapeKey(closeUserCard);
if (!userId) return null;
// key={userId} so switching straight from one card to another (tapping a different @mention
// before closing the first) resets the query/local state instead of showing stale data mid-fetch.
@@ -29,8 +29,16 @@ export function UserCardRoot() {
function UserCardPanel({ userId }: { userId: string }) {
const navigate = useNavigate();
// Registered here, not in the always-mounted UserCardRoot above: useEscapeKey's stack (lib/hooks.ts)
// orders entries by MOUNT time, so an entry registered once at app boot would sit permanently at
// the bottom, below every modal that mounts later -- a real, reproduced bug (open GroupMembersModal
// -> tap a member row to open this card on top of it -> Escape closed the member-list modal
// underneath instead of this card). UserCardPanel only mounts while a card is genuinely open, same
// as every other overlay's own useEscapeKey call, so this now stacks correctly.
useEscapeKey(closeUserCard);
const myUserId = useSessionStore((s) => s.myUserId);
const isSelf = !!myUserId && myUserId === userId;
const [addingFriend, setAddingFriend] = useState(false);
const query = useQuery({
queryKey: ['user-profile', userId],
@@ -38,6 +46,18 @@ function UserCardPanel({ userId }: { userId: string }) {
});
const profile = query.data;
const name = profile ? profile.remark || profile.display_name : '';
// Real bug this fixes: this card's own doc comment always claimed "same relationship-aware action
// gating" as FriendProfileView.tsx, but until now it never read `profile.relationship` at all --
// tapping a non-friend's avatar (this card is reachable from many more places than the friends-only
// surfaces FriendProfileView is, e.g. any group member's avatar or an @mention) showed an enabled
// "发消息"/call row with no add-friend affordance anywhere, gated only by `capabilities.can_send`/
// `can_call` -- a signal ConversationView.tsx's own comment already documents as unreliable for
// exactly this purpose (it 403s from a blocked party's own side instead of reporting false). Mirrors
// FriendProfileView's own isFriend/isPendingOutbound/isPendingInbound branching exactly, just at
// this card's single (not two-size-variant) button size.
const isFriend = profile?.relationship === 'friend';
const isPendingOutbound = profile?.relationship === 'pending_outbound';
const isPendingInbound = profile?.relationship === 'pending_inbound';
const goToFullProfile = () => {
closeUserCard();
@@ -45,6 +65,7 @@ function UserCardPanel({ userId }: { userId: string }) {
};
return (
<>
<div className="fixed inset-0 z-[100] flex items-center justify-center bg-[#22262d]/25" onClick={closeUserCard}>
<div className="w-[300px] overflow-hidden rounded-xl bg-white shadow-modal" onClick={(e) => e.stopPropagation()}>
<div className="flex items-center justify-end px-3 pt-3">
@@ -88,6 +109,8 @@ function UserCardPanel({ userId }: { userId: string }) {
{!isSelf ? (
<div className="mt-5 flex w-full items-center gap-2">
{isFriend ? (
<>
<button
type="button"
disabled={profile.capabilities?.can_send === false}
@@ -124,6 +147,28 @@ function UserCardPanel({ userId }: { userId: string }) {
>
<Icon name="videocam" className="text-base" />
</button>
</>
) : isPendingOutbound ? (
<button type="button" disabled className="h-9 flex-1 rounded-lg bg-btn-disabled text-sm font-medium text-white">
等待验证
</button>
) : isPendingInbound ? (
<button type="button" disabled className="h-9 flex-1 rounded-lg bg-btn-disabled text-sm font-medium text-white">
对方已申请加你
</button>
) : profile.capabilities?.can_add_friend === false ? (
<button type="button" disabled className="h-9 flex-1 rounded-lg bg-btn-disabled text-sm font-medium text-white">
对方暂不可添加
</button>
) : (
<button
type="button"
onClick={() => setAddingFriend(true)}
className="h-9 flex-1 rounded-lg bg-btn text-sm font-medium text-white shadow-btn-sm"
>
添加好友
</button>
)}
</div>
) : null}
@@ -134,5 +179,21 @@ function UserCardPanel({ userId }: { userId: string }) {
)}
</div>
</div>
{/* A true sibling of the card's own backdrop above, not nested inside it: AddFriendModal is
itself a dismissible `fixed inset-0` overlay with its own backdrop-click-to-close handler,
and React events bubble -- nesting it inside the card's backdrop (which ALSO closes on any
click) would make clicking outside AddFriendModal close both overlays at once instead of
just the top one. */}
{addingFriend ? (
<AddFriendModal
onClose={() => setAddingFriend(false)}
initialTarget={
profile
? { user_id: profile.user_id, display_name: profile.display_name, public_account: profile.public_account, avatar_file_id: profile.avatar_file_id }
: undefined
}
/>
) : null}
</>
);
}
+12 -1
View File
@@ -12,6 +12,7 @@ import { GroupAnnouncementModal } from '@/features/groups/GroupAnnouncementModal
import { GroupFilesModal } from '@/features/groups/GroupFilesModal';
import { GroupMembersModal } from '@/features/groups/GroupMembersModal';
import {
canRemoveMember,
disbandGroup,
getGroup,
getMyGroupProfile,
@@ -242,6 +243,14 @@ export function GroupDetailColumn({ topicId, peerName, onClose, onOpenHistory }:
const members = membersQuery.data?.items || [];
const memberCount = groupQuery.data?.member_count ?? settingsQuery.data?.member_count ?? 0;
// Real gap this fixes: `isAdmin` alone gates the "-移出" tile below, but canRemoveMember (the
// per-row check the tile's own destination list uses) is strictly finer -- an admin can't remove
// the owner or another admin, only the owner can remove an admin. A group with no plain members
// (e.g. [owner, admin(me), admin(other)]) would show the tile yet land on a list with zero "移出"
// buttons anywhere, a dead end. Checked against the same preview page the compact grid above
// already renders from (not exhaustive for a group with more members than that page holds, but
// that's the same tradeoff this component's own member grid already makes, not a new one).
const canRemoveAnyone = isAdmin && members.some((m) => canRemoveMember(myUserId, myRole || 'member', m));
return (
<aside className="flex h-full w-full flex-none flex-col overflow-hidden border-l border-border-panel bg-white md:w-[320px] md:overflow-y-auto lg:w-[300px]">
@@ -396,13 +405,15 @@ export function GroupDetailColumn({ topicId, peerName, onClose, onOpenHistory }:
<span className="text-[11px] text-text-secondary">邀请</span>
</button>
) : null}
{isAdmin ? (
{canRemoveAnyone ? (
// Sibling tile to "+邀请" right above -- same dashed-square treatment, mirrored icon.
// A single standalone entry point into the plain member list (not a per-avatar corner
// badge, and not a direct remove action) -- the actual "移出" control already lives on
// each removable row inside that list. Explicit user correction after a first attempt
// put a "-" badge on every removable avatar's own corner: that read as "many separate
// remove buttons" when the intent was one "+"-shaped entry point, mirroring "+邀请".
// Gated on canRemoveAnyone (not bare isAdmin) so the tile doesn't show in a group made
// entirely of owner+admins with nobody actually removable -- a dead end otherwise.
<button type="button" onClick={() => setOpenModal('members')} className="flex flex-col items-center gap-1">
<span className="grid h-11 w-11 place-items-center rounded-lg border border-dashed border-[#c5cad2] text-text-tertiary-3 md:h-10 md:w-10">
<Icon name="remove" className="text-[18px]" />
@@ -30,8 +30,8 @@ interface GroupMembersModalProps {
* from the same settings query) rather than re-deriving from a bare role here, which is exactly
* what silently made that governance toggle a no-op before this was wired through. */
canInvite: boolean;
/** Own user id + role -- drives per-row remove-member eligibility (canRemoveMember below), mirroring
* the backend's own canRemoveIMGroupMember rule exactly rather than re-guessing it client-side. */
/** Own user id + role -- drives per-row remove-member eligibility (canRemoveMember, lib/api/groups.ts),
* mirroring the backend's own canRemoveIMGroupMember rule exactly rather than re-guessing it client-side. */
myUserId: string | null;
myRole: GroupRole;
/** "invite" opens straight into the invite-candidates view, skipping the member list entirely --
@@ -852,8 +852,14 @@ export function ConversationView() {
// (it reads fine in the bubble via renderDrafty's own flatTextContentOf fallback -- only this
// handler's `Drafty.toPlainText` didn't know about it), so "复制" silently put the literal
// string "undefined" on the clipboard instead of the text visibly shown on screen.
// Final `?? ''` is real, not paranoia: `availableActions` only excludes 'copy' for a mobile
// attachment whose payload passes `mobileAttachmentContentOf`'s own validation (needs a real
// `file_id`/`ref`) -- a malformed/partial attachment payload that fails that check falls
// through to here with neither a `.text` nor a `.txt` field, and `Drafty.toPlainText` would
// return `undefined` again, which `writeText` coerces to the literal string "undefined" same
// as the original bug -- this stops that regardless of exactly which shape slips through.
const flatText = flatTextContentOf(msg.content);
navigator.clipboard.writeText(flatText ?? Drafty.toPlainText(msg.content ?? '')).catch(() => {});
navigator.clipboard.writeText(flatText ?? Drafty.toPlainText(msg.content ?? '') ?? '').catch(() => {});
break;
}
case 'recall':
@@ -97,6 +97,15 @@ function SnapshotContent({ item, highlightQuery }: { item: SearchHit; highlightQ
return <span className="text-text-body">{label}</span>;
}
if (contactCardContentOf(asMessage)) return <span className="text-text-body">[名片]</span>;
// Any OTHER flat shape this app doesn't recognize yet (a future/unknown x-im-type, or corrupted
// data) still has neither `fmt`/`ent` (so `Drafty.isPlainText` says "plain text") nor `txt` (so
// `Drafty.toPlainText` returns `undefined`) -- same failure shape as the crash this whole fix
// addresses, just degrading to a silently blank line (highlightMatches' own defensive guard turns
// it into `[]`) instead of throwing. Surface that rather than leaving zero visual signal that
// something is there but unsupported, same spirit as the `(内容不可用)` placeholder above.
if (typeof content === 'object' && !('txt' in content) && !('fmt' in content) && !('ent' in content)) {
return <span className="text-text-tertiary-4">[暂不支持的消息类型]</span>;
}
return <div className="text-sm text-text-body">{renderDrafty(content, highlightQuery)}</div>;
}
+19
View File
@@ -39,6 +39,24 @@ export function onReauthenticated(callback: () => void): () => void {
return () => reconnectAuthListeners.delete(callback);
}
/**
* Fired at the top of resetTinode() (logout/"切换企业"/kicked). Same "plain listener set, not a
* direct import" reasoning as onReauthenticated above -- topics.ts's own module-level caches
* (confirmedReadSeq, previewPrefetched, messageSnapshots, goneTopicNames, chatListSnapshot) used to
* outlive a logout entirely (real, reproduced bug: 'slf' is the same literal topic name for every
* account, so confirmedReadSeq['slf'] from account A surviving into account B's session -- switched
* to in the SAME tab, no full reload -- silently marked B's genuinely-unread self-chat messages as
* read the next time reconcileMeSubs() ticked). This module has no reason to know what "reset" means
* for topics.ts's own state, only that resetTinode() is the one place every teardown path (logout,
* switch-tenant, kicked) already funnels through.
*/
const resetListeners = new Set<() => void>();
export function onTinodeReset(callback: () => void): () => void {
resetListeners.add(callback);
return () => resetListeners.delete(callback);
}
function setStatus(next: ConnectionStatus) {
if (status === next) return;
if (next === 'authenticated') wasAuthenticated = true;
@@ -304,6 +322,7 @@ export function getTinode(): Tinode {
* since those are keyed by nothing but "is there currently a pending one" -- confirmed reachable via
* TenantEntryPage.tsx's "切换企业" flow racing a slow-to-respond old host. */
export function resetTinode() {
resetListeners.forEach((cb) => cb());
tinode?.disconnect();
tinode = null;
connectPromise = null;
+34 -14
View File
@@ -41,26 +41,46 @@ function flatForwardPayload(msg: StoredMessage): { content: DraftyContent | Reco
// A real Drafty document (this app's own composer never produces any of the flat shapes below)
// always has `ent`/`fmt` -- bail out to the normal Drafty path rather than misclassifying it.
if ('ent' in c || 'fmt' in c) return null;
if (typeof (c as { text?: unknown }).text === 'string') {
// Flat plain text (mobile-sent, or a group-announcement auto-post) -- rebuild real Drafty from
// the extracted string instead of preserving the flat object verbatim, so it reads and searches
// like any other text message on this shape's next hop.
return { content: Drafty.forwardedContent(Drafty.init((c as { text: string }).text)), headOverrides: {} };
}
if (typeof headType === 'string' && (headType === 'contact_card' || headType === 'image' || headType === 'file' || headType === 'voice' || headType === 'video')) {
// Flat attachment/contact-card content -- Drafty has no equivalent entity to rebuild (these were
// never real Drafty entities to begin with), so preserve the original content AND its `x-im-type`
// tag verbatim; the receiving client's own dispatch (mobileAttachmentContentOf/
// contactCardContentOf) already renders this shape correctly regardless of which topic it's in.
// Head-gated shapes (attachment/card/call) are checked BEFORE the ungated flat-text shape below,
// matching the check order every other consumer of these shapes uses (preview.ts, MessageBubble.tsx)
// -- no current real shape has both a `.text` and a `file_id`/`user_id`/`callId`, but keeping this
// file's own order the same as theirs means a future shape that combines both (an image with a
// caption, say) forwards as the richer shape instead of silently degrading to caption-only text.
if (typeof headType === 'string' && (headType === 'image' || headType === 'file' || headType === 'voice' || headType === 'video')) {
// Mirrors mobileAttachmentContentOf's own validity check (dispatch.ts) -- an attachment payload
// that's missing both isn't safely forwardable either (nothing on the far end could resolve it),
// so fall through to the ordinary Drafty path rather than preserving known-broken content verbatim.
const attachment = c as { file_id?: unknown; ref?: unknown };
if (typeof attachment.file_id === 'string' || typeof attachment.ref === 'string') {
// Flat attachment content -- Drafty has no equivalent entity to rebuild (this was never a real
// Drafty entity to begin with), so preserve the original content AND its `x-im-type` tag
// verbatim, but as a fresh shallow copy: the SDK's own outbound `simplify()` (see this file's
// other comments on it) mutates a `{pub}` packet's content IN PLACE, deleting falsy fields --
// returning `c` itself would silently strip fields off the SOURCE message's own still-cached
// content too, a real object-ownership bug confirmed by reading `simplify`'s actual
// implementation, not hypothetical.
const headOverrides: Record<string, unknown> = { 'x-im-type': headType };
if (typeof msg.head?.mime === 'string') headOverrides.mime = msg.head.mime;
return { content: c as Record<string, unknown>, headOverrides };
return { content: { ...(c as Record<string, unknown>) }, headOverrides };
}
}
if (headType === 'contact_card' && typeof (c as { user_id?: unknown }).user_id === 'string') {
// Mirrors contactCardContentOf's own validity check (dispatch.ts). Cloned for the same
// in-place-mutation reason as the attachment branch above.
return { content: { ...(c as Record<string, unknown>) }, headOverrides: { 'x-im-type': 'contact_card' } };
}
if (typeof msg.head?.webrtc === 'string' && typeof (c as { callId?: unknown }).callId === 'string') {
// A call-result message -- forwarding one is a low-value edge case (nobody has asked for it),
// but preserving it verbatim is still strictly better than silently losing it the way every
// other flat shape here used to.
return { content: c as Record<string, unknown>, headOverrides: { webrtc: msg.head.webrtc } };
// other flat shape here used to. Cloned for the same in-place-mutation reason as above.
return { content: { ...(c as Record<string, unknown>) }, headOverrides: { webrtc: msg.head.webrtc } };
}
if (typeof (c as { text?: unknown }).text === 'string') {
// Flat plain text (mobile-sent, or a group-announcement auto-post) -- rebuild real Drafty from
// the extracted string instead of preserving the flat object verbatim, so it reads and searches
// like any other text message on this shape's next hop. Builds a fresh object (Drafty.init), so
// no cloning concern here.
return { content: Drafty.forwardedContent(Drafty.init((c as { text: string }).text)), headOverrides: {} };
}
return null;
}
+20 -1
View File
@@ -15,7 +15,7 @@ import type { Contact, StoredMessage, Topic } from 'tinode-sdk';
import { getGroup } from '@/lib/api/groups';
import { MESSAGES_PAGE } from '@/lib/config';
import { getTinode, onReauthenticated } from './connection';
import { getTinode, onReauthenticated, onTinodeReset } from './connection';
const topicListeners = new Map<string, Set<() => void>>();
const anyTopicListeners = new Set<() => void>();
@@ -676,6 +676,25 @@ onReauthenticated(() => {
}
});
/**
* Real, reproduced bug (2026-09-25 review): every module-level cache below is scoped by topic NAME,
* not by account -- and topic names collide across accounts by construction, not edge case (`'slf'`
* is the literal self-chat topic name for every account, `isChatListTopicName` below). Without this,
* logging out and into a DIFFERENT account in the same tab (no full reload -- resetTinode() itself
* never reloads the page) carries the previous account's cached state forward: `confirmedReadSeq`
* specifically was confirmed to silently mark the new account's genuinely-unread 'slf' messages as
* read the next time reconcileMeSubs() ticked, since it read the OLD account's higher confirmed seq
* as a floor for the new account's much-lower real one. Same reasoning extends to every other cache
* here: none of them should outlive the session that populated them.
*/
onTinodeReset(() => {
confirmedReadSeq.clear();
previewPrefetched.clear();
messageSnapshots.clear();
goneTopicNames.clear();
chatListSnapshot = null;
});
const ME_RECONCILE_INTERVAL_MS = 30_000;
let meReconcileTimer: ReturnType<typeof setInterval> | null = null;