fix(sheets): recognize OFL0X local office tokens - #2063
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughSpreadsheet media parent-type logic now recognizes imported office spreadsheets through legacy prefixes or interleaved ChangesOffice spreadsheet parent-type mapping
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
shortcuts/sheets/helpers.go (1)
60-90: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftKeep office-token recognition in one shared implementation.
shortcuts/sheets/helpers.go#L60-L90: make this the single shared classifier.shortcuts/sheets/backward/lark_sheets_float_images.go#L24-L54: remove the duplicate constants/helper and reuse the shared implementation.🤖 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/helpers.go` around lines 60 - 90, Keep isOfficeSpreadsheet in shortcuts/sheets/helpers.go as the sole shared office-token classifier, including its constants and prefix definitions. In shortcuts/sheets/backward/lark_sheets_float_images.go lines 24-54, remove the duplicate constants and helper and update callers to reuse the shared isOfficeSpreadsheet implementation; no direct change is needed in helpers.go beyond preserving it as the shared source.
🤖 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/sheet_media_parent_type_test.go`:
- Around line 45-48: Update the four interleaved non-office token fixtures in
the sheet media parent type tests to be 28 bytes long by appending a filler
byte, while preserving their non-OFL0X markers and expected sheetImageParentType
result.
---
Nitpick comments:
In `@shortcuts/sheets/helpers.go`:
- Around line 60-90: Keep isOfficeSpreadsheet in shortcuts/sheets/helpers.go as
the sole shared office-token classifier, including its constants and prefix
definitions. In shortcuts/sheets/backward/lark_sheets_float_images.go lines
24-54, remove the duplicate constants and helper and update callers to reuse the
shared isOfficeSpreadsheet implementation; no direct change is needed in
helpers.go beyond preserving it as the shared source.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 90a72427-eafb-43f4-b409-80a151b4d69a
📒 Files selected for processing (5)
shortcuts/sheets/backward/lark_sheets_float_images.goshortcuts/sheets/backward/lark_sheets_sheet_media_upload_test.goshortcuts/sheets/helpers.goshortcuts/sheets/sheet_media_parent_type_test.gotests/cli_e2e/sheets/sheets_image_upload_dryrun_test.go
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2063 +/- ##
=======================================
Coverage 75.15% 75.16%
=======================================
Files 912 912
Lines 96453 96477 +24
=======================================
+ Hits 72492 72514 +22
- Misses 18383 18384 +1
- Partials 5578 5579 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
5249125 to
03ea5ce
Compare
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@03ea5cefd584fdab221cf7f3a184d8f832f8e52b🧩 Skill updatenpx skills add larksuite/cli#fix/sheets-ofl0x-office-token -y -g |
Problem
Local Office spreadsheet tokens now use a 28-character interleaved format. The CLI only recognized the legacy
fake_office_andlocal_office_prefixes, so local-image uploads for the new tokens selectedparent_type=sheet_imageinstead of the requiredoffice_sheet_file.Changes
OFL0X.+media-upload,+cells-set-image, and+float-image-create, including native and malformed-token negative cases.Validation
go test -count=1 ./shortcuts/sheets/...go vet ./shortcuts/sheets/..../build.shgo test -count=1 ./tests/cli_e2e/sheets -run TestSheets_ImageUploadDryRunParentTypemake quality-gateSummary by CodeRabbit
Bug Fixes
OFL0Xmarker, ensuring uploads map tooffice_sheet_file(while non-Office/natively handled sheets continue to usesheet_image).Tests
OFL0Xoffice token and revised expectations across media-upload, cells-set-image, and float-image-create flows.