Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

Repository navigation
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

RecordDetailView 切换记录时不清空 feedItems:上一条记录的评论/活动会留在下一条记录的讨论面板上 #3268

Closed
#3284
Closed
RecordDetailView 切换记录时不清空 feedItems:上一条记录的评论/活动会留在下一条记录的讨论面板上#3268
#3284
Assignees
xuyushun441-sys
Labels
bugSomething isn't workingpm:dispatchedpm:queue

Description

@xuyushun441-sys
xuyushun441-sys
opened on Aug 3, 2026

顺路发现于 #3209(那个 PR 只补了 feed 取数的 loading 信号源,没有动 feedItems 的生命周期)。

现象

packages/app-shell/src/views/RecordDetailView.tsx:

  • const [feedItems, setFeedItems] = useState< FeedItem[] >([])(约 L299);
  • 取数 effect 的依赖里有 pureRecordId,换记录时会重新跑两个 find;
  • 但两处回填都是 按 id 归并到 prev:setFeedItems(prev => { const byId = new Map(...); for (const item of [...prev, ...mapped]) ... });
  • 全文件没有任何一处把 feedItems 重置回 [](setFeedItems 只出现在归并与乐观更新里)。

RecordDetailView 在路由下是不会重挂载的(console/AppContent.tsx 里 < RecordDetailView dataSource=... objects=... onEdit=... /> 没有 key;ObjectView / ObjectDataPage / InterfaceListPage 的抽屉用法同理,只换 recordIdOverride),所以在同一个对象里从记录 A 点到记录 B,A 的评论和活动行会继续留在 B 的讨论面板上,B 自己的行归并进来排在一起。id 不同 → Map 去重不掉。

复现证据

在 #3209 的测试装置上跑过一次(临时用例,未提交):

render(< RecordDetailView … recordIdOverride="rec-A" />)
  → 屏幕上出现 "comment for crm_account:rec-A"
rerender(< RecordDetailView … recordIdOverride="rec-B" />)
  → 屏幕上出现 "comment for crm_account:rec-B"
  → 断言 screen.queryByText("comment for crm_account:rec-A") 仍然为真 —— 通过

也就是说 A 的评论确实还在 B 的页面上。

影响

  1. 用户在列表里连续点几条记录,讨论面板会越滚越长,混着别的记录的评论 —— 是串数据,不只是视觉问题;
  2. 顺带压掉 chatter 那条链没有 loading 信号源:RecordDetailView / record:chatter 从不给 RecordChatterPanel 传 loading #3209 的 loading 态:换记录时 feedLoading 会正确转 true,但 RecordActivityTimeline 的 loading 分支条件是 loading && filtered.length === 0(RecordActivityTimeline 收下 loading 却完全不读——取数中的 feed 显示的是"暂无活动" #3205 有意为之:不要让刷新把已在屏上的 feed 变成 spinner),此时 filtered 里还有上一条记录的残留行,于是渲染的是上一条记录的内容而不是加载态。所以这条不修,chatter 那条链没有 loading 信号源:RecordDetailView / record:chatter 从不给 RecordChatterPanel 传 loading #3209 在「记录间导航」这个路径上仍然看不到加载态。

建议方向(未拍板)

取数 effect 里换记录时把 feedItems 清空是最直接的,但要和乐观更新(handleAddComment / handleAddReply / handleToggleReaction 往 feedItems 里塞的本地行)对齐,别把用户刚发出、还没落库的评论清掉。可能的做法是把 feed 状态按 objectName:recordId 键存(和 #3209 的 feedFetchKey 同一把钥匙),读的时候只取当前键那一份 —— 这样「清空」就是自然结果而不是一次额外的 setState 竞态。请维护者定方向,#3209 的 PR 里没有顺手改。

参考

Activity

  1. xuyushun441-sys
    added
    bugSomething isn't working
    needs-user-decision
    pm:queue
    pm:dispatched
    and removed
    needs-user-decision
    on Aug 3, 2026
  2. xuyushun441-sys
    self-assigned this
    on Aug 3, 2026

  3. xuyushun441-sys commented on Aug 3, 2026

    @xuyushun441-sys
    xuyushun441-sys
    on Aug 3, 2026 – with Claude
    ContributorAuthor

    认领:PM 循环第 6 轮 · v17 发布窗口:严重 bug 优先
    会话:session_01NVPjPzmmAJ2Ngtvgg5MSRa(objectui 分片 PM,账号 xuyushun441-sys)
    分支:claude/issue-3268-feed-state-keyed-by-record
    Worktree:objectui-issue-3268

    撤下 needs-user-decision,直接派 —— 因为形态已经被 #3209 定死了

    我此前压这单是因为「清空 vs 按键存」需要拍板,且乐观更新的行让选择有后果。那个前提已经不成立:#3209(PR #3270,已合并 12bf6691e)在同一个文件里落地了 feedFetchKey = \${objectName}:${pureRecordId}``,并用「我要取的键 ≠ 我已结算的键」派生 loading。

    本单正文自己提的方向就是「和 #3209 的 feedFetchKey 同一把钥匙」。所以这不再是二选一 —— 按 objectName:recordId 键存 feed 状态是该文件已确立的惯例,另起一套清空逻辑反而是引入第二种写法(主令 #12)。裁定即此,不上升。

    为什么在 v17 窗口里这单排在前面

    这是串数据,不是视觉瑕疵:用户在列表里连点几条记录,记录 A 的评论和活动会留在记录 B 的讨论面板上,B 自己的行归并进来排在一起,id 不同所以 Map 去重不掉。发布前带着这个走,是"讨论区显示了别的记录的内容"。

    它还压掉了 #3209 刚修好的东西:换记录时 feedLoading 会正确转 true,但 timeline 的分支是 loading && filtered.length === 0(#3205 有意为之,避免刷新把在屏 feed 变 spinner),而残留行让 filtered 非空 —— 于是记录间导航时渲染的是上一条记录的内容而不是加载态。两单合起来才完整。

    硬要求

    1. 按键存,不要额外 setState 清空。 目标是让"清空"成为读取当前键那一份的自然结果,而不是一次与取数竞速的 setFeedItems([])。
    2. ⛔ 不许清掉用户刚发出、尚未落库的乐观行。 handleAddComment / handleAddReply / handleToggleReaction 往 feedItems 里塞的本地行必须跟着它们所属的记录键走 —— 这是本单唯一真正微妙的地方,请在报告里说明你怎么保证的。
    3. 测试必须复现原缺陷:渲染 recordIdOverride="rec-A" → 出现 A 的评论 → rerender 到 rec-B → 断言 A 的评论不再出现。正文里那段临时用例就是模板。另外补一条:换记录时 loading 态能被看见(即 chatter 那条链没有 loading 信号源:RecordDetailView / record:chatter 从不给 RecordChatterPanel 传 loading #3209 在导航路径上也生效)。
    4. mutation 验证:退回归并写法,证明上述两条测试转红。

    ⚠️ scope fence:packages/app-shell/src/views/RecordDetailView.tsx(+ 必要时 packages/plugin-detail 的 timeline 分支,但优先不动它 —— #3205 的 loading && filtered.length === 0 是刻意的,修好 feed 生命周期后它应当自然正确)。同期另有 agent 在 packages/app-shell/src/views/metadata-admin/previews/(#3275),文件不相交,别越界。

    Generated by Claude Code

  4. xuyushun441-sys commented on Aug 3, 2026

    @xuyushun441-sys
    xuyushun441-sys
    on Aug 3, 2026 – with Claude
    ContributorAuthor

    PM 验收:ACCEPT — PR #3284,已入合并队列

    项 结果
    改动范围 ✅ 3 个文件:RecordDetailView.tsx + 新测试 + changeset
    packages/plugin-detail 未动 ✅ —— loading && filtered.length === 0(#3205)原样保留
    CI ✅ 16 项完成,零失败
    按键存而非清空 ✅
    乐观行保住 ✅ 三条链路都有交代

    「一个惯例而不是两个」做到了字面意义上

    feedRecordKey 派生一次,feedFetchKey 再从它派生 —— 文件里只剩一个键表达式,而不是两处各写一遍 ${objectName}:${pureRecordId}。而且它注意到这个键就是行在服务端携带的 thread_id,所以本地分片键与远端归属键是同一个东西,不是巧合对齐。

    比"换记录时清空"更强的一条保证

    两个取数回调都写进各自 effect 闭包里的 threadId,而不是写进"当前记录"。于是一个在用户已经离开之后才返回的响应,会去更新它当初请求的那条记录,永远不可能渗到屏幕上这条。清空式写法给不了这个保证 —— 它只能保证"切换那一刻是空的",迟到的响应照样落到新记录上。测试里专门有一例钉这个(does not let a SLOW response for the record just left overwrite the record now on screen)。

    乐观行的三条链路

    我把这条列为"本任务唯一真正微妙的地方,必须在报告里说明怎么保证的"。它给了三条,缺一不可:

    1. 三个乐观处理器都写进 feedItemsByRecord[feedRecordKey],而 feedRecordKey 与 create('sys_comment', { thread_id }) 用的是同一个表达式 —— 本地行和它未来的持久化副本从一开始就归在同一个键下;三个 useCallback 的依赖数组都补了,handleToggleReaction 还加了 if (!feedRecordKey) return 守卫;
    2. 任何地方都不删分片 —— 离开只是去读另一个键,所以走开再回来还能找到;
    3. mergeFeedRows 是按行 id 求并,回来时的重读会把持久化副本折叠到乐观行上(同一个 id,因为 newItem.id 是显式传给 create 的),结果恰好一行;写入若从未落库(.catch(() => {}) 吞掉),本地行在合并中原样存活。

    顺带一个它指出的副作用:因为不删分片,回到访问过的记录会立刻显示既有行、同时重读在后台确认 —— 这正是 #3205 那条 loading && filtered.length === 0 想要的效果。

    mutation 验证的设计是这单最值得记的部分

    它没有简单地"把改动退回去",而是只退状态键控、保留 feedFetchKey 的记录键控 —— 也就是精确复现修复前的状态,#3209 完好无损。结果:

    Tests  8 failed | 11 passed (19)
× drops record A's comment when the view navigates to record B
× shows the LOADING state on record→record navigation …
AssertionError: expected <p …> to be null          ← A 的评论仍在 B 的面板上
TestingLibraryElementError: Unable to find [data-testid="activity-loading"]  ← 残留行压掉了加载态

    而 #3209 的 11 条测试在该变异下全部保持绿色。 这正是证明新测试钉的是本缺陷、而不是把 #3209 又覆盖一遍的证据。要求 mutation 验证的意义就在这里 —— 它同时回答了"测试能不能红"和"测试红的是不是我以为的那件事"。

    另一处测试设计也对:用 RTL rerender 渲染结构相同的树,让 React 复用同一个 RecordDetailView 实例 —— 注释写明「换成新的 render() 会重挂载,从而把这个 bug 藏起来」。这个 bug 的前提就是"视图不重挂载",测试如果重挂载就永远看不到它。

    消费端未被削弱

    RecordActivityTimeline 的条件一字未改。changeset 里写明了理由:削弱它就是在消费端治标,而 #3165 / #3205 / #3209 一路都刻意避开了这条路。修好 feed 生命周期之后,那个条件自己就是对的。

    Generated by Claude Code

  5. xuyushun441-sys
    closed this as completedin #3284on Aug 3, 2026
  6. akarma-synetal
    added a commit that references this issue on Aug 4, 2026

    fix(app-shell): key the record feed state by objectName:recordId (obj…

    a8aa576
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingpm:dispatchedpm:queue

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions