Repository navigation
fix: fallback on invalid cell conversion in common dataset sync and notify base admins - #915
Conversation
seafile-dev
left a comment
seafile-dev
left a comment
There was a problem hiding this comment.
结论：请求修改。
阻塞问题：无效列集合复用了最多保留 10 行的日志样本，后续行首次出现的错误列不会进入通知，详见行内评论。
警告：目标列名称快照取自源列，目标列重命名后再删除时会展示错误的历史名称，详见行内评论。
已验证：链接公式对象解包以及无法解包时清空单元格并继续追加的兜底路径通过隔离测试；上述两个通知问题均由当前提交中的真实函数复现。隔离测试使用模拟数据库/目标 base API，不代表完整 SeaTable 集成测试。
设计文档要求完整列清单与目标名称快照；建议将本次边界用例补入仓库回归测试。配套 dtable #5810 的通知字段与本 PR 一致，前端应先于或随后端协调发布。
| if rows_invalid_infos: | ||
| logs = [f"dst_dtable_uuid: {dst_dtable_uuid} dst_table_id: {dst_table_id} src_view_id: {src_view_id} some invalid cells warnings"] | ||
| seen_column_keys = set() | ||
| for row_invalid_info in rows_invalid_infos: |
There was a problem hiding this comment.
[Critical] 通知遗漏第十条之后的无效列
影响：rows_invalid_infos 经 append_rows_invalid_infos 限制为最多 10 条，这里却用它生成完整 invalid_columns。当前提交实测：前 10 行仅 A 列无效、第 11 行仅 B 列无效，同步成功且 B 被清空，但返回的通知列只有 A，管理员无法获知 B 的数据丢失。
建议修复：在每个单元格校验时独立累积全量无效列集合，仅对日志行样本保留 10 条限制；补充跨更新/追加批次及第 11 条无效行的回归测试。
| 'src_column_type': col['type'], | ||
| 'dst_column_type': transfered_column.get('type', ColumnTypes.TEXT), | ||
| 'src_column_name': col['name'], | ||
| 'dst_column_name': col['name'], |
There was a problem hiding this comment.
[Warning] 目标列名称快照误用源列名
影响：同步允许目标列保留独立名称，但这里把 col['name'] 同时记录为源、目标名称，新通知又直接将 dst_column_name 保存为 column_name。目标列已重命名时发生转换异常，随后删除该列或子表，配套前端会回退到源列名并标记已删除，而不是设计要求的目标列历史名称；当前函数隔离测试已复现。
建议修复：dst_column_name 使用 transfered_column 的实际名称，并同步修正其余无效值分支；补充目标列重命名后产生通知、再删除列的回归测试。
seafile-dev
left a comment
seafile-dev
left a comment
There was a problem hiding this comment.
结论：通过。当前提交 6fd6c05 已修复上一轮提出的两个问题，未发现新的阻塞缺陷。
原阻塞问题已解决：invalid_columns_dict 独立于最多 10 行的日志样本收集全量无效列，并贯穿更新与追加批次。原名称快照问题也已解决：转换异常、列类型无效及单元格类型无效三个分支均使用目标列名称。已有行内评论对应的缺陷不再适用于当前代码，因此不重复发布。
实际验证：当前生产函数的 8 项隔离回归检查全部通过，包括链接/链接公式对象解包、标量与混合结果、转换异常清空后继续追加、第 11 条错误行、跨两批更新和两批追加的 20,002 行、目标列名称快照、管理员通知字段、重复同步通知以及通知失败不影响同步。数据库与目标 base API 为模拟服务，不代表完整 SeaTable 集成验证。
配套 dtable #5810 字段契约一致；其当前提交原有 31 项 Jest 测试、30 项临时通知组件检查及 src lint 通过。前端复用了已有依赖，并非干净 npm ci 环境；未执行真实浏览器端到端测试。
实现符合设计文档的异常降级、完整无效列通知及历史名称回退要求。建议把这些临时边界检查加入仓库回归测试，并协调前后端发布；设计文档可明确区分限量日志样本与全量通知列集合。
6fd6c05 to
44c7c59
Compare
44c7c59 to
3d37a7b
Compare
seafile-dev
left a comment
seafile-dev
left a comment
There was a problem hiding this comment.
结论：当前范围内的同步崩溃修复通过，未发现新的阻塞问题。
本次评审针对当前提交 3d37a7b。当前 PR 已重新基于 master，仅包含链接/链接公式对象解包、单元格转换异常兜底与诊断日志；上一轮评审提交 44c7c59 不是当前提交的祖先，旧总结中的管理员通知及全量无效列收集不能视为当前实现。此前两条通知相关行内意见不适用于当前范围，不重复发布。
当前生产函数的 8 项隔离回归检查全部通过：对象、标量与混合数组；无法解包对象的异常降级；空值；类型不匹配；其他列类型转换异常；7,659 行更新加 252 行追加；跨两批更新和两批追加的 20,002 行。已在模拟目标写入边界确认异常单元格清空、其他值保留、更新及追加继续、同步返回成功；同一异常输入在当前 base 代码中会抛出 TypeError。诊断日志包含列信息和数据集/同步标识。
设计上下文：开发详情中的 f199f835 智能链接通过认证 API 返回空白正文，无法确认其约束；历史记录中的 235b3baf 文档仍可读取，其对象解包与异常降级要求与当前实现一致，但管理员结构化通知并未出现在当前代码中。因此本次通过结论仅覆盖核心兜底修复与日志，不覆盖无需日志访问即可识别异常列的通知能力。若该能力仍计划交付，应另行明确后端通知与配套前端的 PR 及发布范围。
建议将本次隔离检查纳入仓库回归测试。上述检查使用当前源码提取的原始函数体，数据库和 dtable-server 为模拟服务，未执行完整部署测试或 SeaTable 端到端同步，也未在真实客户元数据上确认最初对象数组的来源。
3 participants
No description provided.