[ISSUE #886] Export filtered topics as CSV - #887
Conversation
There was a problem hiding this comment.
🟡 Not ready to approve
There are confirmed issues in the new CSV formula-injection protection consistency and the test’s global URL mocking configurability that can break other tests depending on execution order.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR fixes Topic export on the Studio Topic management page by generating and downloading a real CSV file for the currently filtered Topic list, addressing Issue #886’s “success toast without artifact” problem.
Changes:
- Add CSV column mapping, CSV escaping, and client-side download via
Blob+URL.createObjectURLin the Topic page. - Update the export button to download the filtered Topic rows and keep the existing success toast.
- Add a regression test ensuring export downloads only the filtered rows and validates CSV escaping / cleanup behavior.
File summaries
| File | Description |
|---|---|
| web/src/pages/instance/topic.tsx | Implements CSV building + download for filtered Topic export. |
| web/src/pages/instance/tests/TopicPage.test.tsx | Adds regression coverage validating filtered export + CSV escaping and download flow. |
Review details
Suppressed comments (1)
web/src/pages/instance/tests/TopicPage.test.tsx:59
- 同上：URL.revokeObjectURL 的 defineProperty 未设置 configurable:true，可能影响其他测试文件在同一 worker 中重新 mock。建议与其他测试保持一致显式设置 configurable:true。
Object.defineProperty(URL, 'revokeObjectURL', {
writable: true,
value: vi.fn(),
});
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| const text = value == null ? '' : String(value); | ||
| const formulaSafeText = /^[=+\-@]/.test(text) ? `'${text}` : text; | ||
| return `"${formulaSafeText.replace(/"/g, '""')}"`; |
| Object.defineProperty(URL, 'createObjectURL', { | ||
| writable: true, | ||
| value: vi.fn(() => 'blob:topic-export'), | ||
| }); |
|
Superseded by consolidated PR #927, which contains this change and has passing checks. Closing this narrower PR to keep review focused. |
Summary
Tests
Closes #886