feat(media): high-level media message builders from UploadResponse - #764
Merged
Conversation
Sending media required hand-assembling the proto (image_message: Some(Box::new(
ImageMessage { url, direct_path, media_key, file_sha256, file_enc_sha256,
file_length, ... })) ) with the CDN/crypto fields spread across the call site.
Add a media module with image_message / video_message / document_message /
audio_message builders that take an UploadResponse plus a typed options struct and
fill the CDN fields (incl. media_key_timestamp, and streaming_sidecar for
video/audio) in one place. Mirrors WA Web's send-media path, which builds the proto
from the upload result internally.
Refactor Status::send_image/send_video to use the builders (also propagating
media_key_timestamp + the streaming sidecar, which they previously dropped) and
delete the now-unused UploadResponse::*_vec adapters.
Tests: each builder maps the CDN fields, defaults the mimetype, and carries the
sidecar/type-specific options.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds a public ChangesMedia Message Builders
Sequence Diagram(s)sequenceDiagram
participant Status
participant UploadResponse
participant image_message
participant video_message
participant wa_Message
Status->>UploadResponse: supply upload metadata (url, path, keys, hashes, length, sidecar)
Status->>image_message: call with ImageOptions (caption, jpeg_thumbnail)
image_message->>wa_Message: build ImageMessage with CDN/crypto fields, caption, mimetype default
Status->>video_message: call with VideoOptions (caption, jpeg_thumbnail, duration, gif_playback)
video_message->>wa_Message: build VideoMessage with streaming_sidecar, seconds, gif_playback, CDN/crypto fields
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
You should review the media builders first, then the status call sites, then the upload helper removal. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. Comment |
Benchmark Results67 unchanged benchmark(s)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
The example called client.upload(&bytes, "image/jpeg") but upload takes (Vec<u8>, MediaType, UploadOptions), so the no_run doctest failed to compile and broke Build & Test (which runs doctests; the --all-targets/--lib jobs skip them). Replace it with a minimal builder-only example that doesn't depend on the upload/send signatures.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: edb43737c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the api-29 gap.
Sending media meant hand-assembling the proto —
image_message: Some(Box::new(ImageMessage { url, direct_path, media_key, file_sha256, file_enc_sha256, file_length, ... }))— with the CDN/crypto fields spread across every call site (andStatus::send_image/send_videoeach duplicated ~9 of those assignments).Adds a
mediamodule withimage_message/video_message/document_message/audio_messagebuilders that take anUploadResponseplus a typed options struct and fill the CDN fields in one place — includingmedia_key_timestampand, for video/audio, thestreaming_sidecarfrom the upload. Mirrors WA Web's send-media path, which builds the proto from the upload result internally.Status::send_image/send_videoare refactored to use the builders (so they now also propagatemedia_key_timestamp+ the streaming sidecar, which they previously dropped), and the now-unusedUploadResponse::media_key_vec/file_sha256_vec/file_enc_sha256_vecadapters are deleted.Usage:
Builders are pure functions (no client needed), so they're trivially testable. Tests cover the CDN-field mapping, mimetype defaulting, the sidecar carry-through, and the type-specific options for all four media types.
Scope note: the builder lives in the high-level crate (not
wacore::proto_helpersas the finding suggested) becauseUploadResponseis defined there;wacorecan't depend on it.