Skip to content

fix(im): preserve original URL filename for uploaded file messages#514

Merged
tuxedomm merged 1 commit intomainfrom
fix/im-url-filename
Apr 16, 2026
Merged

fix(im): preserve original URL filename for uploaded file messages#514
tuxedomm merged 1 commit intomainfrom
fix/im-url-filename

Conversation

@tuxedomm
Copy link
Copy Markdown
Collaborator

@tuxedomm tuxedomm commented Apr 16, 2026

Summary

  • mediaBuffer.FileName() was hardcoded to "media" + ext, so IM file messages sent via URL displayed generic names like media.pdf instead of the filename parsed from the URL. The refactor that introduced mediaBuffer inadvertently threw away the basename that fileNameFromURL(rawURL) already provided.
  • Store fileNameFromURL(rawURL) on the buffer and return it from FileName(). Split newMediaBuffer so the URL-to-filename wiring is reachable from tests without going through the hardened download transport.
  • Add a lock-in test for the local upload branch to confirm filepath.Base(filePath) stays the file_name field, so a future consolidation of URL and local paths cannot silently regress local uploads.

Test plan

  • go test -race ./shortcuts/im/...
  • golangci-lint run ./shortcuts/im/... — no new issues (two pre-existing gofmt -s hints remain from f5a8fbf; out of scope for this fix)
  • New unit tests:
    • TestMediaBufferFileName — direct struct contract
    • TestNewMediaBufferFromBytesURLFilename — URL → mediaBuffer.name wiring across path / query / percent-encoded / fallback / non-http cases
    • TestUploadFileToIMPreservesLocalFileName — asserts the multipart file_name field for local uploads

Summary by CodeRabbit

  • Bug Fixes

    • Uploaded files retain their original local filenames instead of receiving synthetic names.
    • Filenames derived from URLs are correctly decoded (percent-encoded spaces), strip query strings, and fall back to a sensible default when needed.
  • Tests

    • Added tests covering local filename preservation and URL-derived filename extraction behavior.

@github-actions github-actions Bot added domain/im PR touches the im domain size/M Single-domain feat or fix with limited business impact labels Apr 16, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 16, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 833f6ae0-6e6d-4f32-86f3-65b3e4f4dd5f

📥 Commits

Reviewing files that changed from the base of the PR and between 89ef7ec and 9962675.

📒 Files selected for processing (3)
  • shortcuts/im/coverage_additional_test.go
  • shortcuts/im/helpers.go
  • shortcuts/im/helpers_network_test.go
✅ Files skipped from review due to trivial changes (2)
  • shortcuts/im/helpers_network_test.go
  • shortcuts/im/coverage_additional_test.go

📝 Walkthrough

Walkthrough

Filename preservation added: mediaBuffer now stores an original name derived from source URLs; newMediaBuffer delegates to newMediaBufferFromBytes(...) which wires name; FileName() returns the stored name. Tests added for URL-derived names and for preserving local upload basenames.

Changes

Cohort / File(s) Summary
Media buffer implementation
shortcuts/im/helpers.go
Added name field to mediaBuffer; introduced newMediaBufferFromBytes(data, ext, rawURL) and refactored newMediaBuffer to use it; FileName() now returns b.name (URL/local-derived filename) instead of a synthetic name.
Unit tests — filename extraction
shortcuts/im/coverage_additional_test.go
Added tests verifying mediaBuffer.FileName() behavior and newMediaBufferFromBytes URL-to-name extraction, percent-decoding, query-stripping, and fallback to "download".
Network test — upload payload
shortcuts/im/helpers_network_test.go
Added test TestUploadFileToIMPreservesLocalFileName that stubs IM upload endpoint and asserts multipart file_name contains the local file's basename.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

size/L

Suggested reviewers

  • YangJunzhou-01

Poem

🐰
I dug for names in every URL trail,
Decoded spaces, stripped queries without fail.
Local files hop in with their true-bare name,
Tests nibble proofs, nothing left to claim.
Hooray — no more synthetic fame!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: preserving the original URL filename for uploaded file messages instead of using a generic 'media' prefix.
Description check ✅ Passed The pull request description includes all required sections: Summary (explaining the problem and solution), Changes (implicit in the summary), Test Plan (with completed checkboxes and specific test names), and Related Issues.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/im-url-filename

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

mediaBuffer.FileName() returned a hardcoded "media"+ext, so IM file
messages sent via URL displayed generic names like "media.pdf" instead
of the filename parsed from the URL. This regressed the pre-refactor
tempfile path which at least carried a unique basename.

Store fileNameFromURL(rawURL) on the buffer and return it from
FileName(). Split newMediaBuffer so the URL-to-filename wiring is
reachable from tests without going through the hardened download
transport.

Also lock in that the local upload branch keeps filepath.Base(filePath)
as file_name, so the URL fix cannot silently regress the local branch
later.

Change-Id: I729b217e9dc9237aeb89c2b89df86a37ad64a840
@tuxedomm tuxedomm force-pushed the fix/im-url-filename branch from 89ef7ec to 9962675 Compare April 16, 2026 10:55
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 16, 2026

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@9962675cc186f2e35b8d5fea4e1a1e7fae0476ad

🧩 Skill update

npx skills add larksuite/cli#fix/im-url-filename -y -g

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 16, 2026

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 59.06%. Comparing base (d0ab8ee) to head (9962675).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
shortcuts/im/helpers.go 75.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #514   +/-   ##
=======================================
  Coverage   59.05%   59.06%           
=======================================
  Files         384      384           
  Lines       32632    32634    +2     
=======================================
+ Hits        19270    19274    +4     
+ Misses      11553    11551    -2     
  Partials     1809     1809           

☔ View full report in Codecov by Sentry.
📢 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.

@tuxedomm tuxedomm merged commit 79379fb into main Apr 16, 2026
21 checks passed
@tuxedomm tuxedomm deleted the fix/im-url-filename branch April 16, 2026 11:01
chenjinxiong03-bit pushed a commit to chenjinxiong03-bit/cli that referenced this pull request Apr 17, 2026
…arksuite#514)

mediaBuffer.FileName() returned a hardcoded "media"+ext, so IM file
messages sent via URL displayed generic names like "media.pdf" instead
of the filename parsed from the URL. This regressed the pre-refactor
tempfile path which at least carried a unique basename.

Store fileNameFromURL(rawURL) on the buffer and return it from
FileName(). Split newMediaBuffer so the URL-to-filename wiring is
reachable from tests without going through the hardened download
transport.

Also lock in that the local upload branch keeps filepath.Base(filePath)
as file_name, so the URL fix cannot silently regress the local branch
later.

Change-Id: I729b217e9dc9237aeb89c2b89df86a37ad64a840
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/im PR touches the im domain size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants