feat: JPEG/PNG download choice on shared folder/project pages - #159
Conversation
The format menu shipped in bwhtech#157 only reached the review page and the asset action menu — a guest on a /vms/shared/folder or /vms/shared/:project link could still only download a RAW/HEIC file in its original format. - vms.api.download_shared_converted_asset: guest endpoint, share-token scoped (same validation as get_shared_asset_download_url), 300 MB cap + 15/min rate limit via serve_converted_download - SharedProjectPage: per-asset Download becomes Original / JPEG / PNG for RAW/HEIC stills; "Download all" stays originals - MediaPreviewDialog: optional downloadMenu prop renders the same choice in the preview header Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N6kPRDGo6QbcVcc8DPkMTf
Greptile SummaryThis PR adds JPEG and PNG conversion downloads to shared project and folder pages while preserving original downloads.
Confidence Score: 4/5The PR appears safe to merge, with the non-blocking recommendation to add automated coverage for the new public endpoint’s authorization branches. The endpoint reuses established scope validation and guarded conversion behavior, and the frontend wiring is compatible with existing callers; the only accepted concern is missing regression coverage for the guest-accessible boundary. Files Needing Attention: vms/api.py
|
| Filename | Overview |
|---|---|
| vms/api.py | Adds the scoped guest conversion-download endpoint; implementation follows existing authorization and conversion paths, but lacks endpoint regression tests. |
| frontend/src/pages/SharedProjectPage.vue | Adds conversion menus to shared asset cards and previews using the existing scope and download conventions. |
| frontend/src/components/common/MediaPreviewDialog.vue | Adds an optional frappe-ui download dropdown while retaining the existing plain download button behavior. |
Prompt To Fix All With AI
### Issue 1
vms/api.py:2258
**Guest Endpoint Lacks Tests**
The new guest-accessible endpoint has no automated regression coverage for valid project and folder shares, invalid tokens, or assets outside the shared scope. These authorization branches protect a public download path, so relying only on manual probes makes future scope-validation regressions difficult to detect. Please add focused endpoint tests for successful project and folder downloads and the rejection cases.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "feat: JPEG/PNG download choice on shared..." | Re-trigger Greptile
| # nosemgrep: frappe-semgrep-rules.rules.security.guest-whitelisted-method | ||
| @frappe.whitelist(allow_guest=True) | ||
| @rate_limit(key="asset_name", limit=15, seconds=60, methods=["GET"]) | ||
| def download_shared_converted_asset( |
There was a problem hiding this comment.
The new guest-accessible endpoint has no automated regression coverage for valid project and folder shares, invalid tokens, or assets outside the shared scope. These authorization branches protect a public download path, so relying only on manual probes makes future scope-validation regressions difficult to detect. Please add focused endpoint tests for successful project and folder downloads and the rejection cases.
Prompt To Fix With AI
This is a comment left during a code review.
Path: vms/api.py
Line: 2258
Comment:
**Guest Endpoint Lacks Tests**
The new guest-accessible endpoint has no automated regression coverage for valid project and folder shares, invalid tokens, or assets outside the shared scope. These authorization branches protect a public download path, so relying only on manual probes makes future scope-validation regressions difficult to detect. Please add focused endpoint tests for successful project and folder downloads and the rejection cases.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Problem
Reported: on a
/vms/shared/folder/…?token=…link, a guest downloading a.arwstill gets the.arw— no JPEG/PNG option.The format menu from #157 was wired into the review page and the asset action menu only. The shared folder / shared project page (
SharedProjectPage.vue) has its own download path (vms.api.get_shared_asset_download_url→ presigned original) that #157 never touched.Fix
Backend —
vms.api.download_shared_converted_asset(asset_name, format, project?, token?, folder?)allow_guest, validated with_validate_shared_asset_scope(identical toget_shared_asset_download_url)serve_converted_download→ same 300 MB cap,@rate_limit(15/min, keyed on asset), embedded-preview render for RAW / libheif for HEICFrontend
SharedProjectPage.vue: the per-card Download button becomes Original (ARW) / JPEG / PNG for RAW/HEIC stills (isConvertibleStill); other files and Download all unchanged (originals)MediaPreviewDialog.vue: new optionaldownloadMenuprop — when set, the preview header's Download button renders the same menu; unset keeps the plain link (other callers unaffected)Screenshot
Guest shared-folder page, ARW card:
Verified (local, live site)
DSC02816.jpgyarn typecheck/yarn lint/yarn buildclean;ruff check/ruff formatclean🤖 Generated with Claude Code