-
Notifications
You must be signed in to change notification settings - Fork 961
fix(sheets): add mention_type enum to set_cell_range cells schema#1439
fix(sheets): add mention_type enum to set_cell_range cells schema #1439zhengzhijiej-tech wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughRestricts ChangesMention Type Enum Validation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
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 docstrings
🧪 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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/sheets/lark_sheet_write_cells_test.go`:
- Around line 447-450: Replace the brittle substring-based assertion around
combined := stdout + stderr + err.Error() with typed error assertions: use
errs.ProblemOf(err) to extract problem metadata and assert problem.Category and
problem.Subtype match the expected values (e.g., mention_type / not-in-enum),
and use errors.As(err, new(*errs.ValidationError)) to assert the Param is the
expected field and that the underlying cause is preserved (compare
ProblemOf().Cause or check errors.Is against the original cause). Update the
test around the strings.Contains checks to perform these typed assertions
instead of substring matching.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
i️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 01e3a37b-813d-4abe-9eee-ea45d6106e1b
📒 Files selected for processing (2)
shortcuts/sheets/data/flag-schemas.jsonshortcuts/sheets/lark_sheet_write_cells_test.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use typed error assertions for this error-path test.
This check is message-substring based; for error-path tests here, assert typed metadata (category/subtype) and cause preservation instead. For param, use errors.As(..., *errs.ValidationError) rather than errs.ProblemOf.
Based on learnings and coding guidelines: *_test.go error paths should use errs.ProblemOf + cause checks, and Param must be asserted via errors.As on *errs.ValidationError.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@shortcuts/sheets/lark_sheet_write_cells_test.go` around lines 447 - 450,
Replace the brittle substring-based assertion around combined := stdout + stderr
+ err.Error() with typed error assertions: use errs.ProblemOf(err) to extract
problem metadata and assert problem.Category and problem.Subtype match the
expected values (e.g., mention_type / not-in-enum), and use errors.As(err,
new(*errs.ValidationError)) to assert the Param is the expected field and that
the underlying cause is preserved (compare ProblemOf().Cause or check errors.Is
against the original cause). Update the test around the strings.Contains checks
to perform these typed assertions instead of substring matching.
Sources: Coding guidelines, Learnings
Codecov Report
✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.87%. Comparing base (2a7e9c7) to head (4227973).
Additional details and impacted files
@@ Coverage Diff @@ ## main #1439 +/- ## ========================================== + Coverage 72.83% 72.87% +0.04% ========================================== Files 732 737 +5 Lines 69140 69446 +306 ========================================== + Hits 50356 50609 +253 - Misses 15003 15037 +34 - Partials 3781 3800 +19
☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.
🚀 New features to boost your workflow:
- ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
🚀 PR Preview Install Guide
🧰 CLI update
npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@422797305a05ed7b5ae168f9db35fc401ea1a7a9
🧩 Skill update
npx skills add larksuite/cli#fix/sheet-mention-type-enum -y -g
Constrain rich_text mention_type to the proto MENTION_FILE_TYPE set so a
file @mention with an out-of-enum value (e.g. 6 = cloud shared folder) is
rejected by the schema validator before it reaches the server and fails
pb serialization ("mentionFileInfo.fileType: enum value expected").
- data/flag-schemas.json: mention_type gains enum + per-value description
- lark_sheet_write_cells_test.go: cover reject (6) + allow (0 / 2 / 22)
ab35368 to
4227973
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/sheets/data/flag-schemas.json`:
- Around line 465-479: The enum for the mention type is missing values required
by the proto (add 2, 4, and 5); update the "enum" array tied to the property
whose "description" begins with "`@提及类型编号`(仅 type='mention' 时可选)" so it includes
0,1,2,3,4,5,8,11,12,15,16,22,30,38, ensuring the schema's allowed mention_type
values match the MENTION_FILE_TYPE allowlist and will not reject valid mentions
at parse time.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
i️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: adcdddd7-e99e-4da4-b0c3-b6b0784e7cf2
📒 Files selected for processing (2)
shortcuts/sheets/data/flag-schemas.jsonshortcuts/sheets/lark_sheet_write_cells_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- shortcuts/sheets/lark_sheet_write_cells_test.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Schema enum is missing valid mention_type values required by the proto contract.
At Line 467-Line 478, the enum omits 2, 4, and 5, while the stated MENTION_FILE_TYPE allowlist includes them. Because schema validation is enforced before execution, valid mentions with those values will now be rejected at parse time.
Proposed fix
- "description": "`@提及类型编号`(仅 type='mention' 时可选)。0 或不填=`@用户`;@文件时按类型取:1=文档 3=电子表格 8=多维表格 11=思维笔记 12=文件 15=旧版幻灯片 16=知识库 22=新版文档 30=幻灯片 38=画板", + "description": "`@提及类型编号`(仅 type='mention' 时可选)。0 或不填=`@用户`;@文件时按类型编号取值(详见 enum 列表)。", "type": "number", "enum": [ 0, 1, + 2, 3, + 4, + 5, 8, 11, 12, 15, 16, 22, 30, 38 ]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@shortcuts/sheets/data/flag-schemas.json` around lines 465 - 479, The enum for
the mention type is missing values required by the proto (add 2, 4, and 5);
update the "enum" array tied to the property whose "description" begins with
"`@提及类型编号`(仅 type='mention' 时可选)" so it includes
0,1,2,3,4,5,8,11,12,15,16,22,30,38, ensuring the schema's allowed mention_type
values match the MENTION_FILE_TYPE allowlist and will not reject valid mentions
at parse time.
Uh oh!
There was an error while loading. Please reload this page.
Problem
A
set_cell_rangerich_text @mention with an unsupportedmention_type(e.g.6= cloud shared folder) was copied verbatim into thesetRangeValuesmutation'smentionFileInfo.fileTypeand failed pb serialization (mentionFileInfo.fileType: enum value expected), taking the whole write down. The--cellsschema declaredmention_typeas a barenumberwith no enum.Change
shortcuts/sheets/data/flag-schemas.json:mention_typenow carries anenum— the render-sideMENTION_FILE_TYPEset[0,1,3,8,11,12,15,16,22,30,38](the setcheckIsEmbed/MENTION_TO_SUITErecognize), plus a per-value description. Values that are proto-serializable but absent from this set (FOLDER=2,SHEET_DOC=4,CHAT=5) would survive serialization yet not render — a silent broken mention — so they are excluded too.0/ omitted = a user @mention. Synced from the upstream tool schema.shortcuts/sheets/lark_sheet_write_cells_test.go: reject6, allow0/22.Verification
go test ./shortcuts/sheets/passes.