Skip to main content

max / makenotwork

Answer the version download with a redirect, not JSON describing one GET /api/versions/{id}/download answered JSON carrying a presigned URL and the caller navigated to it, so a described Action::get would have sent the reader to a JSON body. That made the download button the one control on the files tab that could not be said in the description layer, and Shape 3 step 4 closed without it. Ruled (a) rather than serving the route through the description layer: it answers 303 to the presigned URL, so navigating to it is the download. Three of the four response fields retired with the JSON, all measured unread. file_name is already rendered server-side by every surface that shows one, license_url had no reader anywhere in the tree, and expires_in was ignored. download_url was the only field anyone consumed, and a redirect is it. Access control, the scan-status gates and both download counters are untouched. The counters still run before the answer, because this handler is the last point at which the intent to download is observable. The two assertions on the body become assertions on the status and Location. The non-purchaser 403 test is untouched: it asserted a status all along. What this does not do is describe the button. It makes it describable, and it converts with the files tab.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-08-26 14:18 UTC
Signed with PGP, not checked
Commit: 34adcf768f48df68d8e5ca05f2bd6446babdce32
Parent: 067a5b2
8 files changed, +71 insertions, -87 deletions
M server/static/item-upload.js +6 -9
@@ -242,20 +242,17 @@
242 242
243 243 document.querySelectorAll('.download-version-btn').forEach(function(btn) {
244 244 btn.addEventListener('click', function() {
245 - fetch('/api/versions/' + btn.dataset.versionId + '/download')
246 - .then(function(res) {
247 - if (!res.ok) throw new Error('Failed to get download URL');
248 - return res.json();
249 - })
250 - .then(function(data) { window.location.href = data.download_url; })
251 - .catch(function(err) { showToast(err.message); });
245 + window.location.href =
246 + '/api/versions/' + btn.dataset.versionId + '/download';
252 247 });
253 248 });
254 249
255 250 // The delete button is described: `crate::quasi::version_delete_act`,
256 251 // Shape 3 step 4. Route, row, prompt and tone all live on the act, and
257 - // htmx performs it. The download button above stays here because its
258 - // route answers JSON carrying a presigned URL rather than the file.
252 + // htmx performs it. The download button above is now a plain navigation
253 + // because its route answers 303 to the presigned URL (`8fc6b1af`); it is
254 + // describable as an `Action::get` whenever the files tab converts, which
255 + // is what taking the JSON away bought.
259 256
260 257 document.getElementById('cancel-version-upload-btn').addEventListener('click', function() {
261 258 uploader.cancel();
M server/static/page-library-audio.js +4 -11
@@ -1,13 +1,6 @@
1 + // The route answers 303 to a presigned URL, so navigating to it IS the
2 + // download (`8fc6b1af`, option (a)). It used to answer JSON describing the
3 + // URL, which is why this was a fetch-then-assign.
1 4 function downloadVersion(versionId) {
2 - fetch('/api/versions/' + versionId + '/download')
3 - .then(function(res) {
4 - if (!res.ok) throw new Error('Failed to get download URL');
5 - return res.json();
6 - })
7 - .then(function(data) {
8 - window.location.href = data.download_url;
9 - })
10 - .catch(function(err) {
11 - showToast(err.message || 'Download failed');
12 - });
5 + window.location.href = '/api/versions/' + versionId + '/download';
13 6 }
M server/static/page-library-downloads.js +4 -11
@@ -16,16 +16,9 @@
16 16 }
17 17 })();
18 18
19 + // The route answers 303 to a presigned URL, so navigating to it IS the
20 + // download (`8fc6b1af`, option (a)). It used to answer JSON describing the
21 + // URL, which is why this was a fetch-then-assign.
19 22 function downloadVersion(versionId) {
20 - fetch('/api/versions/' + versionId + '/download')
21 - .then(function(res) {
22 - if (!res.ok) throw new Error('Failed to get download URL');
23 - return res.json();
24 - })
25 - .then(function(data) {
26 - window.location.href = data.download_url;
27 - })
28 - .catch(function(err) {
29 - showToast(err.message || 'Download failed');
30 - });
23 + window.location.href = '/api/versions/' + versionId + '/download';
31 24 }
M server/static/page-library-video.js +4 -11
@@ -1,13 +1,6 @@
1 + // The route answers 303 to a presigned URL, so navigating to it IS the
2 + // download (`8fc6b1af`, option (a)). It used to answer JSON describing the
3 + // URL, which is why this was a fetch-then-assign.
1 4 function downloadVersion(versionId) {
2 - fetch('/api/versions/' + versionId + '/download')
3 - .then(function(res) {
4 - if (!res.ok) throw new Error('Failed to get download URL');
5 - return res.json();
6 - })
7 - .then(function(data) {
8 - window.location.href = data.download_url;
9 - })
10 - .catch(function(err) {
11 - showToast(err.message || 'Download failed');
12 - });
5 + window.location.href = '/api/versions/' + versionId + '/download';
13 6 }
M server/src/quasi/version_delete_act.rs +12 -10
@@ -32,17 +32,19 @@
32 32 //! a fact the page does not have. A version row has an id in the database and
33 33 //! the template was simply not writing it down.
34 34 //!
35 - //! # The download button beside it is not here, and that is measured
35 + //! # The download button beside it is sayable now, and was not when this landed
36 36 //!
37 - //! Wiki [[mnw-shape-conversion-plans]] Shape 3 step 4 says
38 - //! `download-version-btn` is `Action::get`. It is not.
39 - //! `GET /api/versions/{id}/download` (`routes::storage::downloads`) answers
40 - //! **JSON carrying a presigned URL**, and the handler then navigates to it, so
41 - //! a described `Action::get` would send the reader to a JSON body. Making it
42 - //! sayable is a route change — answer a redirect, or serve the route through
43 - //! the description layer so it can answer `Outcome::Goto` with a
44 - //! `Destination::External` — rather than a control change, so the handler
45 - //! stays.
37 + //! Wiki [[mnw-shape-conversion-plans]] Shape 3 step 4 said
38 + //! `download-version-btn` is `Action::get`. When this module landed that was
39 + //! wrong: `GET /api/versions/{id}/download` (`routes::storage::downloads`)
40 + //! answered JSON carrying a presigned URL and the handler navigated to it, so a
41 + //! described `Action::get` would have sent the reader to a JSON body.
42 + //!
43 + //! Fixed 2026-08-26 by the route rather than by the control (`8fc6b1af`,
44 + //! option (a)): it answers **303 to the presigned URL**, so navigating to it is
45 + //! the download and `Action::get` says the truth. The button is a plain
46 + //! navigation in `static/item-upload.js` today and becomes an act whenever the
47 + //! files tab converts. Nothing blocks it any more.
46 48
47 49 use makeover_layout::Tone;
48 50 use quasi_router::screen::Act;
M server/tests/workflows/creator_media.rs +4 -4
@@ -292,11 +292,11 @@
292 292 .client
293 293 .get(&format!("/api/versions/{version_id}/download"))
294 294 .await;
295 - assert_eq!(resp.status, 200, "Download failed: {}", resp.text);
296 - let data: Value = resp.json();
295 + // 303 to the presigned URL, not JSON describing it (`8fc6b1af`, option (a)).
296 + assert_eq!(resp.status, 303, "Download failed: {}", resp.text);
297 297 assert!(
298 - data["download_url"].is_string(),
299 - "Should return download_url"
298 + resp.headers.contains_key("location"),
299 + "303 should carry the presigned URL in Location"
300 300 );
301 301 }
302 302
M server/tests/workflows/storage.rs +14 -6
@@ -469,13 +469,21 @@
469 469 .client
470 470 .get(&format!("/api/versions/{version_id}/download"))
471 471 .await;
472 - assert_eq!(resp.status, 200, "Version download failed: {}", resp.text);
473 - let data: Value = resp.json();
472 + // 303 to the presigned URL, not JSON describing it (`8fc6b1af`, option (a)).
473 + assert_eq!(
474 + resp.status, 303,
475 + "Version download should redirect: {}",
476 + resp.text
477 + );
478 + let location = resp
479 + .headers
480 + .get("location")
481 + .expect("303 carries a Location")
482 + .to_str()
483 + .unwrap();
474 484 assert!(
475 - data["download_url"]
476 - .as_str()
477 - .unwrap()
478 - .starts_with("http://test-storage/")
485 + location.starts_with("http://test-storage/"),
486 + "Location should be the presigned URL, got: {location}"
479 487 );
480 488 }
481 489
M server/src/routes/storage/downloads.rs +23 -25
@@ -3,7 +3,7 @@
3 3 use axum::{
4 4 Json,
5 5 extract::{Path, State},
6 - response::IntoResponse,
6 + response::{IntoResponse, Redirect},
7 7 };
8 8 use serde::Serialize;
9 9 use sqlx::PgPool;
@@ -23,16 +23,6 @@
23 23 pub expires_in: u64,
24 24 }
25 25
26 - /// JSON response containing a presigned download URL for a version.
27 - #[derive(Debug, Serialize)]
28 - pub(super) struct VersionDownloadResponse {
29 - pub download_url: String,
30 - pub file_name: Option<String>,
31 - pub expires_in: u64,
32 - #[serde(skip_serializing_if = "Option::is_none")]
33 - pub license_url: Option<String>,
34 - }
35 -
36 26 /// Resolve a content URL for downloadable media. Always a presigned, expiring
37 27 /// URL, even for free content.
38 28 ///
@@ -165,11 +155,27 @@
165 155 }))
166 156 }
167 157
168 - /// Generate a presigned URL for downloading a version file
158 + /// Redirect to a presigned URL for downloading a version file
169 159 ///
170 160 /// GET /api/versions/{version_id}/download
171 161 ///
172 162 /// Access control: free items are accessible to anyone, paid items require purchase.
163 + ///
164 + /// Answers **303 See Other** to the presigned URL rather than JSON describing it.
165 + /// Ruled 2026-08-26 (`8fc6b1af`, option (a)): a described `Action::get` sends the
166 + /// reader wherever the route sends them, so a JSON body made this control the one
167 + /// thing on the files tab that could not be said in the description layer.
168 + ///
169 + /// Three fields retired with the JSON, all measured unread: `file_name` (every
170 + /// surface already renders it server-side from `version.file_name`), `license_url`
171 + /// (no reader anywhere in the tree) and `expires_in` (the expiry is 3600 either
172 + /// way). `download_url` was the only one anyone consumed, and a redirect *is* it.
173 + ///
174 + /// A redirect is a webview fact, so a terminal or egui host cannot perform it.
175 + /// Serving this route through the description layer so it can answer
176 + /// `Outcome::Goto` with a `Destination::External` remains the cross-host spelling,
177 + /// and stays available: it waits on the `QUASI_SCREENS` flip chain (`64b33b26`),
178 + /// which is why it was not chosen now.
173 179 #[tracing::instrument(skip_all, name = "storage::version_download", fields(version_id))]
174 180 pub(super) async fn version_download(
175 181 State(db): State<PgPool>,
@@ -228,7 +234,7 @@
228 234 }
229 235 }
230 236
231 - let (download_url, expires_in) = resolve_content_url(s3.as_ref(), s3_key, 3600).await?;
237 + let (download_url, _expires_in) = resolve_content_url(s3.as_ref(), s3_key, 3600).await?;
232 238
233 239 // Increment per-version and item-level download counts
234 240 db::versions::increment_download_count(&db, version_id).await?;
@@ -239,16 +245,8 @@
239 245 let _ = db::versions::record_user_download(&db, user.id, version.item_id, version_id).await;
240 246 }
241 247
242 - let license_url = if item.license_preset.is_some() {
243 - Some(format!("/api/items/{}/license.txt", version.item_id))
244 - } else {
245 - None
246 - };
247 -
248 - Ok(Json(VersionDownloadResponse {
249 - download_url,
250 - file_name: version.file_name,
251 - expires_in,
252 - license_url,
253 - }))
248 + // The counters above run before the redirect for the same reason they ran
249 + // before the JSON: this handler is the only place the intent to download is
250 + // observable. Once the reader is at the storage host we never hear about it.
251 + Ok(Redirect::to(&download_url))
254 252 }