fix(attachments): normalize attachment list handling and add test cases - #334
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughUpdates make UrlAttachment.size optional, add URL/name parsing utilities, refine file-type detection and URL normalization, and change the items watcher to always recompute the file list. New test pages and Playwright specs validate multiple attachment scenarios. Documentation reflects the optional size change. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 44 minutes and 14 seconds.Comment |
SonyLeo
changed the title
|
✅ Preview build completed successfully!
|
📦 Package Previewpnpm add https://pkg.pr.new/@opentiny/tiny-robot@78843bd pnpm add https://pkg.pr.new/@opentiny/tiny-robot-kit@78843bd pnpm add https://pkg.pr.new/@opentiny/tiny-robot-svgs@78843bd commit: 78843bd |
…ta normalization and rendering
f235e9c to
f39c9c6
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/test/src/attachments/index.spec.ts (1)
69-75: Add an explicitundefined-items clearing testLines 69-75 validate clearing to empty, but this PR also touches behavior when
itemsbecomesundefined. Adding that case would lock down the watcher change and prevent regressions.Suggested additional test
test('父级将 items 置为 undefined 后应同步清空附件列表', async ({ page }) => { const section = page.locator('[data-testid="url-only-section"]') await expect(section.locator('.tr-file-card')).toHaveCount(1) await page.getByTestId('url-only-set-undefined').click() // add control in test page if missing await expect(section.locator('.tr-file-card')).toHaveCount(0) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/test/src/attachments/index.spec.ts` around lines 69 - 75, Add a new Playwright test that mirrors the existing empty-clear case but sets the parent `items` to undefined and asserts attachments are cleared: create a test named like '父级将 items 置为 undefined 后应同步清空附件列表' that queries the same section locator '[data-testid="url-only-section"]', asserts the initial `.tr-file-card` count is 1, clicks a control with test id 'url-only-set-undefined' (add that control to the test page if it doesn't exist), then asserts `.tr-file-card` count is 0 to lock down watcher behavior when `items` becomes undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/components/src/attachments/utils.ts`:
- Line 1: The FILE_NAME_QUERY_KEYS array currently includes the generic key
'name' which causes functions like getStringDetectionCandidates() and
getUrlDisplayName() to treat query params such as ?name=alice as filenames;
remove 'name' from FILE_NAME_QUERY_KEYS so it only contains 'filename' and
'fileName' (i.e., update the const FILE_NAME_QUERY_KEYS to exclude 'name') and
run/update any tests relying on the broader behavior; references to
FILE_NAME_QUERY_KEYS in getStringDetectionCandidates() and getUrlDisplayName()
will then correctly prefer path-based filenames or explicit filename keys.
In `@packages/test/src/attachments/index.spec.ts`:
- Around line 4-8: The setup uses a broad h2 locator in test.beforeEach which
can match unrelated headings; tighten the assertion by targeting the specific
heading text or role—replace the
expect(page.locator('h2')).toContainText('Attachments 组件测试') with a more
specific locator (e.g., getByRole('heading' with name "Attachments 组件测试" and
level 2, or a locator scoped under the panel opened by
page.click('text=Attachments 组件')) so the test.beforeEach reliably asserts the
intended heading.
---
Nitpick comments:
In `@packages/test/src/attachments/index.spec.ts`:
- Around line 69-75: Add a new Playwright test that mirrors the existing
empty-clear case but sets the parent `items` to undefined and asserts
attachments are cleared: create a test named like '父级将 items 置为 undefined
后应同步清空附件列表' that queries the same section locator
'[data-testid="url-only-section"]', asserts the initial `.tr-file-card` count is
1, clicks a control with test id 'url-only-set-undefined' (add that control to
the test page if it doesn't exist), then asserts `.tr-file-card` count is 0 to
lock down watcher behavior when `items` becomes undefined.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: edc3ee7c-511d-4330-b65c-b0fbfcd78fac
📒 Files selected for processing (9)
docs/src/components/attachments.mdpackages/components/src/attachments/composables/useFileType.tspackages/components/src/attachments/index.type.tspackages/components/src/attachments/index.vuepackages/components/src/attachments/utils.tspackages/test/src/App.vuepackages/test/src/attachments/index.spec.tspackages/test/src/attachments/index.vuepackages/test/src/home/index.vue
SonyLeo
changed the title
🧹 Preview Cleaned UpThe preview deployment has been removed. |
2 participants
问题
normalizeAttachments在处理输入数据时存在两个缺陷：items.map()回调只处理了isUrlSizeItem（有 url + size）和isRawFileItem（有 rawFile）两种情况，不符合任一条件的 item 返回undefined，被as Attachment[]强转掩盖。isUrlSizeItem判定过严：要求typeof item.size === 'number'，导致只传url不传size的远程附件（这是合理的使用方式）无法被识别，落入无人处理的分支变成undefined。此外，
watch中if (newItems && newItems.length > 0)的守卫导致父组件清空items时fileList不会同步清空。改动
commit 1
5f5c01bd— 核心修复（5 个文件）useFileType.ts：map改为reduce，只有匹配到的 item 才 push 进结果，不匹配的跳过而非返回undefinedisRawFileItem判定优先于isUrlItem（有 rawFile 的即使同时带 url 也走本地文件路径）isUrlItem去掉size的强制要求，只要有url即可transformUrlItem中：用getUrlDisplayName(url)替代简单的url.split('/').pop()，支持从 query 参数提取文件名；item.fileType ?? inferredFileType尊重用户显式传入的 fileTypetransformRawFileItem中：item.fileType ?? detectFileType(rawFile)同样尊重用户传入值；item.size ?? rawFile.size用??替代||index.type.ts+docs/src/components/attachments.md：UrlAttachment.size从number改为number?，类型与逻辑对齐index.vue：newItems.length > 0守卫，改为normalizeAttachments(newItems || [])，父组件清空时同步清空utils.ts（新增）：getStringDetectionCandidates：为 URL 字符串生成多个候选值（原始值、query 参数中的 filename、pathname、最后一段路径），供detectFileType逐一匹配getUrlDisplayName：从 URL 中提取人类可读的文件名，优先级为 query 参数 > 最后一段路径 > 完整路径commit 2
f39c9c6f— E2E 测试（4 个文件）packages/test/src/attachments/index.vue测试页面，覆盖 7 种数据输入场景packages/test/src/attachments/index.spec.ts，8 个 Playwright 测试用例测试覆盖
Summary by CodeRabbit
Bug Fixes
New Features