Skip to main content

max / goingson

The attachments column picks a file, and opens one Closes the porting half of 844b5ae0. The column shipped read-only because neither control was a route; both are sayable now, so both are here. Picking is a FieldKind::File field posting to POST /projects/{id}/attachments, which is the part that had to be built rather than described: a picked file says nothing about where the bytes go. The handler calls attach_path, lifted out of the add_attachment command so the hashing, the dedup and the size limit have one copy. Opening spools the blob out under its own filename and answers Response::goto(Action::external("file://...")). A refusal comes back on the field the file came from, so picking the wrong thing costs one click rather than a reload. AttachFailure keeps a Missing arm so the command still answers a bad parent id with a 404. The transport half is the host's and is not settled: Tauri can hand over a path, a browser sends multipart, which quasi-http deliberately does not read. Recorded on attachments_column.
Author: Max Johnson <me@maxj.phd> · 2026-08-09 16:37 UTC
Signed with PGP, not checked
Commit: d8d46324939df73a3a8f5174868abb45fb4c2b7e
Parent: ac39d07
3 files changed, +398 insertions, -203 deletions
M src-tauri/src/commands/attachment.rs +257 -184
@@ -64,123 +64,183 @@
64 64 pub has_local_blob: bool,
65 65 }
66 66
67 - // Commands
68 -
69 - /// Attach a file to a task or project.
67 + /// Why attaching or spooling a file stopped.
70 68 ///
71 - /// Reads the file, computes SHA-256, copies to blob store (dedup), and creates
72 - /// the attachment record. The `file_path` comes from the JS file picker dialog.
73 - #[tauri::command]
74 - #[instrument(skip_all)]
75 - pub async fn add_attachment(
76 - state: State<'_, Arc<AppState>>,
69 + /// Two arms rather than one string, because the two callers say a refusal
70 + /// differently and neither can tell which it was from the text. A command
71 + /// answers with `ApiError`; the described attachments column puts a refusal back
72 + /// on the field the file came from and lets a failure be a 500. The split is
73 + /// whose fault it is: `Refused` is the user's and is fixable by picking another
74 + /// file, `Failed` is ours.
75 + #[derive(Debug)]
76 + pub(crate) enum AttachFailure {
77 + /// The file will not do, and saying so is the whole answer.
78 + Refused(String),
79 + /// What it was to be attached to is not there.
80 + ///
81 + /// Its own arm rather than a [`Refused`](Self::Refused) with a sentence in
82 + /// it, because the command answered a bad parent id with a 404 before this
83 + /// was lifted out and still has to. A caller that has no route to a
84 + /// not-found — the described column, whose project came from the address it
85 + /// was already rendering — reads it as a refusal.
86 + Missing {
87 + /// What was looked for.
88 + resource: &'static str,
89 + /// Which one.
90 + id: String,
91 + },
92 + /// Something on our side did not work.
93 + Failed(String),
94 + }
95 +
96 + impl From<AttachFailure> for ApiError {
97 + fn from(failure: AttachFailure) -> Self {
98 + match failure {
99 + AttachFailure::Refused(message) => Self::validation("filePath", message),
100 + AttachFailure::Missing { resource, id } => Self::not_found(resource, id),
101 + AttachFailure::Failed(message) => Self::internal(message),
102 + }
103 + }
104 + }
105 +
106 + impl AttachFailure {
107 + /// The sentence to put in front of the user, whichever arm it is.
108 + pub(crate) fn message(&self) -> String {
109 + match self {
110 + Self::Refused(message) | Self::Failed(message) => message.clone(),
111 + Self::Missing { resource, id } => format!("{resource} not found: {id}"),
112 + }
113 + }
114 +
115 + /// The same failure said as a bad request rather than a complaint about a
116 + /// field. What the two open paths answer with: they are addressed at an
117 + /// attachment that exists, not at a file the user chose, so there is no
118 + /// field for the message to hang on.
119 + pub(crate) fn bad_request(self) -> ApiError {
120 + match self {
121 + Self::Failed(message) => ApiError::internal(message),
122 + other => ApiError::bad_request(other.message()),
123 + }
124 + }
125 + }
126 +
127 + /// Attach a file that is already on disk, synchronously.
128 + ///
129 + /// The body of [`add_attachment`], lifted out of the command 2026-08-09 so the
130 + /// described project dashboard's `POST /projects/{id}/attachments` can call the
131 + /// same code rather than a second copy of it. A `quasi_router` handler is a
132 + /// plain `fn(&S, Params)` with nothing async about it, so this is sync and the
133 + /// command is what puts it on the blocking pool.
134 + ///
135 + /// Reads the file, computes SHA-256, copies to the blob store (dedup), and
136 + /// creates the attachment record.
137 + pub(crate) fn attach_path(
138 + state: &AppState,
77 139 task_id: Option<TaskId>,
78 140 project_id: Option<ProjectId>,
79 - file_path: String,
80 - ) -> Result<AttachmentResponse, ApiError> {
141 + file_path: &str,
142 + ) -> Result<goingson_core::Attachment, AttachFailure> {
81 143 // Validate at least one parent
82 144 if task_id.is_none() && project_id.is_none() {
83 - return Err(ApiError::validation_msg(
84 - "Either taskId or projectId is required",
145 + return Err(AttachFailure::Refused(
146 + "Either a task or a project is required".to_owned(),
85 147 ));
86 148 }
87 149
88 150 // Verify the parent exists *before* touching the blob store, so a bad id
89 151 // fails fast instead of after a disk write orphans a blob.
90 - if let Some(tid) = task_id {
91 - state
152 + if let Some(tid) = task_id
153 + && state
92 154 .tasks
93 - .get_by_id(tid, DESKTOP_USER_ID)?
94 - .or_not_found("task", tid)?;
155 + .get_by_id(tid, DESKTOP_USER_ID)
156 + .map_err(|error| AttachFailure::Failed(error.to_string()))?
157 + .is_none()
158 + {
159 + return Err(AttachFailure::Missing {
160 + resource: "task",
161 + id: tid.to_string(),
162 + });
95 163 }
96 - if let Some(pid) = project_id {
97 - state
164 + if let Some(pid) = project_id
165 + && state
98 166 .projects
99 - .get_by_id(pid, DESKTOP_USER_ID)?
100 - .or_not_found("project", pid)?;
167 + .get_by_id(pid, DESKTOP_USER_ID)
168 + .map_err(|error| AttachFailure::Failed(error.to_string()))?
169 + .is_none()
170 + {
171 + return Err(AttachFailure::Missing {
172 + resource: "project",
173 + id: pid.to_string(),
174 + });
101 175 }
102 176
103 - let source_path = Path::new(&file_path);
177 + let source_path = Path::new(file_path);
104 178
105 179 // Validate path exists and is a file
106 180 if !source_path.is_file() {
107 - return Err(ApiError::validation(
108 - "filePath",
109 - "File does not exist or is not a regular file",
181 + return Err(AttachFailure::Refused(
182 + "File does not exist or is not a regular file".to_owned(),
110 183 ));
111 184 }
112 185
113 186 // Validate no path traversal
114 187 if file_path.contains("..") {
115 - return Err(ApiError::validation(
116 - "filePath",
117 - "Path traversal not allowed",
188 + return Err(AttachFailure::Refused(
189 + "Path traversal not allowed".to_owned(),
118 190 ));
119 191 }
120 192
121 193 // Read file metadata
122 194 let metadata = std::fs::metadata(source_path)
123 - .map_api_err("Failed to read file metadata", ApiError::internal)?;
195 + .map_err(|error| AttachFailure::Failed(format!("Failed to read file metadata: {error}")))?;
124 196
125 197 if metadata.len() > MAX_FILE_SIZE {
126 - return Err(ApiError::validation(
127 - "filePath",
128 - format!(
129 - "File too large ({}, max {})",
130 - format_file_size(metadata.len() as i64),
131 - format_file_size(MAX_FILE_SIZE as i64),
132 - ),
133 - ));
198 + return Err(AttachFailure::Refused(format!(
199 + "File too large ({}, max {})",
200 + format_file_size(metadata.len() as i64),
201 + format_file_size(MAX_FILE_SIZE as i64),
202 + )));
134 203 }
135 204
136 - // Read (up to MAX_FILE_SIZE), hash, and write the blob on the blocking pool so a
137 - // large attachment doesn't stall the async reactor (ultra-fuzz Run #27 Perf S1).
138 - let source_owned = source_path.to_path_buf();
205 + // Bound the read itself. The `fs::metadata` check above is a separate
206 + // syscall, so a file that grows in between (or a symlink swapped for a
207 + // larger target — `is_file()` follows symlinks) would otherwise be read
208 + // whole into memory, past the limit that check exists to enforce. Taking
209 + // one byte more than the limit is what distinguishes "at the limit" from
210 + // "over it".
211 + use std::io::Read as _;
212 + let handle = std::fs::File::open(source_path)
213 + .map_err(|error| AttachFailure::Failed(format!("Failed to read file: {error}")))?;
214 + let mut file_data = Vec::new();
215 + handle
216 + .take(MAX_FILE_SIZE + 1)
217 + .read_to_end(&mut file_data)
218 + .map_err(|error| AttachFailure::Failed(format!("Failed to read file: {error}")))?;
219 + if file_data.len() as u64 > MAX_FILE_SIZE {
220 + return Err(AttachFailure::Refused(format!(
221 + "File too large (max {})",
222 + format_file_size(MAX_FILE_SIZE as i64)
223 + )));
224 + }
225 + let file_size = file_data.len() as i64;
226 + let hash = {
227 + let mut hasher = Sha256::new();
228 + hasher.update(&file_data);
229 + hex::encode(hasher.finalize())
230 + };
231 +
139 232 let blobs_dir = state.data_dir.join("blobs");
140 - let (hash, file_size, wrote_new_blob, blob_path) = tokio::task::spawn_blocking(
141 - move || -> Result<(String, i64, bool, std::path::PathBuf), String> {
142 - // Bound the read itself. The `fs::metadata` check above is a separate
143 - // syscall, so a file that grows in between (or a symlink swapped for a
144 - // larger target — `is_file()` follows symlinks) would otherwise be read
145 - // whole into memory, past the limit that check exists to enforce. Taking
146 - // one byte more than the limit is what distinguishes "at the limit" from
147 - // "over it".
148 - use std::io::Read as _;
149 - let handle = std::fs::File::open(&source_owned)
150 - .map_err(|e| format!("Failed to read file: {e}"))?;
151 - let mut file_data = Vec::new();
152 - handle
153 - .take(MAX_FILE_SIZE + 1)
154 - .read_to_end(&mut file_data)
155 - .map_err(|e| format!("Failed to read file: {e}"))?;
156 - if file_data.len() as u64 > MAX_FILE_SIZE {
157 - return Err(format!(
158 - "File too large (max {})",
159 - format_file_size(MAX_FILE_SIZE as i64)
160 - ));
161 - }
162 - let file_size = file_data.len() as i64;
163 - let hash = {
164 - let mut hasher = Sha256::new();
165 - hasher.update(&file_data);
166 - hex::encode(hasher.finalize())
167 - };
168 - std::fs::create_dir_all(&blobs_dir)
169 - .map_err(|e| format!("Failed to create blobs directory: {e}"))?;
170 - // Copy to blob store (skip if hash already exists, dedup). Track whether
171 - // we wrote it so a failed insert can roll back exactly the blob we added.
172 - let blob_path = blobs_dir.join(&hash);
173 - let wrote_new_blob = !blob_path.exists();
174 - if wrote_new_blob {
175 - std::fs::write(&blob_path, &file_data)
176 - .map_err(|e| format!("Failed to write blob: {e}"))?;
177 - }
178 - Ok((hash, file_size, wrote_new_blob, blob_path))
179 - },
180 - )
181 - .await
182 - .map_err(|e| ApiError::internal(format!("Attachment task panicked: {e}")))?
183 - .map_err(ApiError::internal)?;
233 + std::fs::create_dir_all(&blobs_dir).map_err(|error| {
234 + AttachFailure::Failed(format!("Failed to create blobs directory: {error}"))
235 + })?;
236 + // Copy to blob store (skip if hash already exists, dedup). Track whether
237 + // we wrote it so a failed insert can roll back exactly the blob we added.
238 + let blob_path = blobs_dir.join(&hash);
239 + let wrote_new_blob = !blob_path.exists();
240 + if wrote_new_blob {
241 + std::fs::write(&blob_path, &file_data)
242 + .map_err(|error| AttachFailure::Failed(format!("Failed to write blob: {error}")))?;
243 + }
184 244
185 245 // Extract filename from path
186 246 let filename = source_path
@@ -191,30 +251,110 @@
191 251
192 252 let mime_type = mime_from_extension(&filename).to_string();
193 253
194 - let create_result = state.attachments.create(
195 - DESKTOP_USER_ID,
196 - NewAttachment {
197 - task_id,
198 - project_id,
199 - filename,
200 - file_size,
201 - mime_type,
202 - blob_hash: hash,
203 - source_email_id: None,
204 - },
205 - );
206 -
207 - let attachment = match create_result {
208 - Ok(a) => a,
209 - Err(e) => {
254 + state
255 + .attachments
256 + .create(
257 + DESKTOP_USER_ID,
258 + NewAttachment {
259 + task_id,
260 + project_id,
261 + filename,
262 + file_size,
263 + mime_type,
264 + blob_hash: hash,
265 + source_email_id: None,
266 + },
267 + )
268 + .map_err(|error| {
210 269 // The row never landed; reclaim the blob we just wrote so it doesn't
211 270 // leak. Only remove a blob we created this call (dedup hits must stay).
212 271 if wrote_new_blob {
213 272 let _ = std::fs::remove_file(&blob_path);
214 273 }
215 - return Err(e.into());
216 - }
217 - };
274 + AttachFailure::Failed(error.to_string())
275 + })
276 + }
277 +
278 + /// Copy one blob out to the temp spool under its own filename, and say where.
279 + ///
280 + /// What both open paths do before handing a file to something else, and what
281 + /// the described dashboard's open route answers with as a `file://` address.
282 + /// Sync for the reason [`attach_path`] is; the commands wrap it.
283 + ///
284 + /// The spool is keyed by hash prefix so two attachments with the same filename
285 + /// do not overwrite each other, and both the directory and the file are made
286 + /// owner-only before the bytes land: the system temp dir is world-readable and
287 + /// a decrypted blob must not be.
288 + pub(crate) fn spool(
289 + data_dir: &Path,
290 + blob_hash: &str,
291 + filename: &str,
292 + ) -> Result<PathBuf, AttachFailure> {
293 + if !is_valid_blob_hash(blob_hash) {
294 + return Err(AttachFailure::Refused(
295 + "Invalid attachment reference".to_owned(),
296 + ));
297 + }
298 + let blob = blob_path(data_dir, blob_hash);
299 + if !blob.exists() {
300 + return Err(AttachFailure::Refused(
301 + "Blob not available locally; sync required".to_owned(),
302 + ));
303 + }
304 +
305 + let hash_prefix = &blob_hash[..8];
306 + let temp_dir = std::env::temp_dir()
307 + .join("goingson-attachments")
308 + .join(hash_prefix);
309 + std::fs::create_dir_all(&temp_dir)
310 + .map_err(|error| AttachFailure::Failed(format!("Failed to create temp dir: {error}")))?;
311 + // Owner-only before the copy: the decrypted blob must not be readable by
312 + // other local users via the world-readable system temp dir.
313 + super::harden_temp_dir(&temp_dir);
314 +
315 + let temp_path = temp_dir.join(safe_filename(filename));
316 + std::fs::copy(&blob, &temp_path)
317 + .map_err(|error| AttachFailure::Failed(format!("Failed to copy file: {error}")))?;
318 + super::harden_temp_file(&temp_path);
319 +
320 + Ok(temp_path)
321 + }
322 +
323 + /// A filename with everything that could leave the spool directory taken out.
324 + fn safe_filename(filename: &str) -> String {
325 + let safe: String = filename
326 + .replace(['/', '\\'], "_")
327 + .replace("..", "_")
328 + .chars()
329 + .filter(|c| !c.is_control())
330 + .collect();
331 + if safe.is_empty() {
332 + "attachment".to_owned()
333 + } else {
334 + safe
335 + }
336 + }
337 +
338 + // Commands
339 +
340 + /// Attach a file to a task or project.
341 + ///
342 + /// The work is [`attach_path`], on the blocking pool so a large attachment
343 + /// doesn't stall the async reactor (ultra-fuzz Run #27 Perf S1). The
344 + /// `file_path` comes from the JS file picker dialog.
345 + #[tauri::command]
346 + #[instrument(skip_all)]
347 + pub async fn add_attachment(
348 + state: State<'_, Arc<AppState>>,
349 + task_id: Option<TaskId>,
350 + project_id: Option<ProjectId>,
351 + file_path: String,
352 + ) -> Result<AttachmentResponse, ApiError> {
353 + let owned = Arc::clone(&state);
354 + let attachment =
355 + tokio::task::spawn_blocking(move || attach_path(&owned, task_id, project_id, &file_path))
356 + .await
357 + .map_err(|e| ApiError::internal(format!("Attachment task panicked: {e}")))??;
218 358
219 359 Ok(to_response(attachment, &state.data_dir))
220 360 }
@@ -268,50 +408,15 @@
268 408 .get_by_id(id, DESKTOP_USER_ID)?
269 409 .or_not_found("attachment", id)?;
270 410
271 - if !is_valid_blob_hash(&attachment.blob_hash) {
272 - return Err(ApiError::bad_request("Invalid attachment reference"));
273 - }
274 - let blob_path = state.data_dir.join("blobs").join(&attachment.blob_hash);
275 - if !blob_path.exists() {
276 - return Err(ApiError::bad_request(
277 - "Blob not available locally; sync required",
278 - ));
279 - }
280 -
281 - // Create a temp directory keyed by blob hash so different attachments with the same
282 - // filename don't overwrite each other.
283 - let hash_prefix = if attachment.blob_hash.len() >= 8 {
284 - &attachment.blob_hash[..8]
285 - } else {
286 - &attachment.blob_hash
287 - };
288 - let temp_dir = std::env::temp_dir()
289 - .join("goingson-attachments")
290 - .join(hash_prefix);
291 - std::fs::create_dir_all(&temp_dir)
292 - .map_api_err("Failed to create temp dir", ApiError::internal)?;
293 - // Owner-only before the copy: the decrypted blob must not be readable by
294 - // other local users via the world-readable system temp dir.
295 - super::harden_temp_dir(&temp_dir);
296 -
297 - // Sanitize filename: strip path separators, .., and control characters to prevent path traversal
298 - let safe_name: String = attachment
299 - .filename
300 - .replace(['/', '\\'], "_")
301 - .replace("..", "_")
302 - .chars()
303 - .filter(|c| !c.is_control())
304 - .collect();
305 - let safe_name = if safe_name.is_empty() {
306 - "attachment".to_string()
307 - } else {
308 - safe_name
309 - };
310 - let temp_path = temp_dir.join(&safe_name);
311 - // Copy blob to temp with original filename (overwrite if exists). On the blocking
312 - // pool, a large attachment copy must not stall the reactor (Perf S1).
313 - copy_blocking(blob_path, temp_path.clone()).await?;
314 - super::harden_temp_file(&temp_path);
411 + // On the blocking pool, a large attachment copy must not stall the reactor
412 + // (Perf S1).
413 + let data_dir = state.data_dir.clone();
414 + let temp_path = tokio::task::spawn_blocking(move || {
415 + spool(&data_dir, &attachment.blob_hash, &attachment.filename)
416 + })
417 + .await
418 + .map_err(|e| ApiError::internal(format!("Spool task panicked: {e}")))?
419 + .map_err(AttachFailure::bad_request)?;
315 420
316 421 open::that(&temp_path).map_api_err("Failed to open file", ApiError::internal)?;
317 422
@@ -465,43 +570,11 @@
465 570 blob_hash: String,
466 571 filename: String,
467 572 ) -> Result<(), ApiError> {
468 - if !is_valid_blob_hash(&blob_hash) {
469 - return Err(ApiError::bad_request("Invalid attachment reference"));
470 - }
471 - let blob_path = state.data_dir.join("blobs").join(&blob_hash);
472 - if !blob_path.exists() {
473 - return Err(ApiError::bad_request(
474 - "Attachment not available locally; sync required",
475 - ));
476 - }
477 -
478 - let hash_prefix = if blob_hash.len() >= 8 {
479 - &blob_hash[..8]
480 - } else {
481 - &blob_hash
482 - };
483 - let temp_dir = std::env::temp_dir()
484 - .join("goingson-attachments")
485 - .join(hash_prefix);
486 - std::fs::create_dir_all(&temp_dir)
487 - .map_api_err("Failed to create temp dir", ApiError::internal)?;
488 - super::harden_temp_dir(&temp_dir);
489 -
490 - let safe_name: String = filename
491 - .replace(['/', '\\'], "_")
492 - .replace("..", "_")
493 - .chars()
494 - .filter(|c| !c.is_control())
495 - .collect();
496 - let safe_name = if safe_name.is_empty() {
497 - "attachment".to_string()
498 - } else {
499 - safe_name
500 - };
501 - let temp_path = temp_dir.join(&safe_name);
502 -
503 - copy_blocking(blob_path, temp_path.clone()).await?;
504 - super::harden_temp_file(&temp_path);
573 + let data_dir = state.data_dir.clone();
574 + let temp_path = tokio::task::spawn_blocking(move || spool(&data_dir, &blob_hash, &filename))
575 + .await
576 + .map_err(|e| ApiError::internal(format!("Spool task panicked: {e}")))?
577 + .map_err(AttachFailure::bad_request)?;
Lines truncated
M src-tauri/src/quasi/projects/dashboard.rs +21 -14
@@ -25,14 +25,18 @@
25 25 //! - `GET /projects/{id}/dashboard` — the whole thing.
26 26 //! - `POST /projects/{id}/milestones/{milestone}/move` — reorder, `by=-1|1`.
27 27 //! - `POST /projects/{id}/milestones/{milestone}/delete` — delete one.
28 + //! - `POST /projects/{id}/attachments` — attach the picked file.
29 + //! - `GET /projects/{id}/attachments/{attachment}/open` — hand one to the host.
28 30 //!
29 31 //! `showCompletedMilestones` is module state in the JS, re-rendered from a
30 32 //! cached list. Here it is `?completed=1`, so the expanded dashboard is
31 33 //! reachable by address. Third time decision 2 has paid out on a real screen and
32 34 //! the first where the state was a disclosure rather than a filter.
33 35 //!
34 - //! Two controls are described and two are not, and the line between them is the
35 - //! finding this port turned up. See [`attachments_column`].
36 + //! Two controls were left out when this screen first landed, because neither a
37 + //! file picker nor a handoff to the OS was anything an action could reach. Both
38 + //! are here now, and what closing that took is the finding this port turned up.
39 + //! See [`attachments_column`].
36 40
37 41 #![allow(clippy::needless_pass_by_value)]
38 42
@@ -483,15 +487,14 @@
483 487 makeover_layout::Tone::Success,
484 488 format!("Attached {}.", attachment.filename),
485 489 )),
486 - // A refusal is the user's to fix by picking another file, so it goes
487 - // back on the field. A failure is ours and is not something a form can
488 - // say anything useful about.
489 - Err(crate::commands::attachment::AttachFailure::Refused(message)) => {
490 - attachments_pane(state, id, Some(&message))
491 - }
490 + // A failure is ours and is not something a form can say anything useful
491 + // about. Everything else is the user's to fix by picking another file,
492 + // so it goes back on the field — including the project having gone,
493 + // which is a stale screen rather than a 404 worth navigating to.
492 494 Err(crate::commands::attachment::AttachFailure::Failed(message)) => {
493 495 Err(RouteError::internal(message))
494 496 }
497 + Err(failure) => attachments_pane(state, id, Some(&failure.message())),
495 498 }
496 499 }
497 500
@@ -524,14 +527,12 @@
524 527 &attachment.filename,
525 528 )
526 529 .map_err(|failure| match failure {
527 - // Not there yet rather than not there at all: an unsynced blob is a
528 - // thing the user can wait for, and the message says which it is.
529 - crate::commands::attachment::AttachFailure::Refused(message) => {
530 - RouteError::not_found(message)
531 - }
532 530 crate::commands::attachment::AttachFailure::Failed(message) => {
533 531 RouteError::internal(message)
534 532 }
533 + // Not there yet rather than not there at all: an unsynced blob is a
534 + // thing the user can wait for, and the message says which it is.
535 + other => RouteError::not_found(other.message()),
535 536 })?;
536 537
537 538 Ok(Response::goto(Action::external(file_url(&spooled))))
@@ -544,12 +545,18 @@
544 545 /// the separator kept. A filename with a space or a `#` in it is the common
545 546 /// case this exists for, and both would otherwise truncate the address.
546 547 fn file_url(path: &std::path::Path) -> String {
548 + use std::fmt::Write as _;
549 +
547 550 let mut url = String::from("file://");
548 551 for byte in path.to_string_lossy().bytes() {
549 552 match byte {
550 553 b'/' | b'-' | b'.' | b'_' | b'~' => url.push(byte as char),
551 554 _ if byte.is_ascii_alphanumeric() => url.push(byte as char),
552 - _ => url.push_str(&format!("%{byte:02X}")),
555 + // Infallible into a `String`, and the one thing a `?` here could
556 + // report is that formatting failed, which it cannot.
557 + _ => {
558 + let _ = write!(url, "%{byte:02X}");
559 + }
553 560 }
554 561 }
555 562 url
M src-tauri/src/quasi/projects/dashboard/tests.rs +120 -5
@@ -385,16 +385,131 @@
385 385 }
386 386
387 387 #[tokio::test]
388 - async fn the_attachment_controls_are_absent_because_they_are_not_routes() {
389 - // The finding. `attachments.pickAndAttach` opens the OS file picker, which
390 - // is neither a route this app answers nor an external address a browser
391 - // navigates to. Left out rather than pointed at a route that cannot exist.
388 + async fn the_attachments_column_offers_a_file_field() {
389 + // The finding, closed. `attachments.pickAndAttach` opens the OS file
390 + // picker, which was neither a route this app answers nor an external
391 + // address; `FieldKind::File` is how the description asks for a file without
392 + // naming one host's way of choosing it.
392 393 let state = state().await;
393 394 let project = project(&state);
394 395 let page = dashboard(&state, project);
395 396
396 397 assert!(page.contains("No attachments yet."));
397 - assert!(!page.contains("Attach File"));
398 + assert!(page.contains(r#"type="file""#));
399 + assert!(page.contains("Attach file"));
400 + }
401 +
402 + /// A file on disk to attach, named for the test that wants it.
403 + fn a_file(name: &str, contents: &str) -> std::path::PathBuf {
404 + let dir = std::env::temp_dir().join("goingson-quasi-attach-tests");
405 + std::fs::create_dir_all(&dir).unwrap();
406 + let path = dir.join(name);
407 + std::fs::write(&path, contents).unwrap();
408 + path
409 + }
410 +
411 + fn attach(state: &AppState, project: ProjectId, path: &std::path::Path) -> Response {
412 + post(
413 + state,
414 + &format!("/projects/{project}/attachments"),
415 + Params::new().with("file", path.to_str().unwrap()),
416 + )
417 + }
418 +
419 + #[tokio::test]
420 + async fn attaching_a_picked_file_answers_with_the_column_it_landed_in() {
421 + let state = state().await;
422 + let project = project(&state);
423 + let response = attach(&state, project, &a_file("notes.txt", "hello"));
424 +
425 + // The column alone, not the whole screen: attaching lands in one place, so
426 + // an expanded milestones section survives it.
427 + assert_eq!(response.target(), Some("dashboard-attachments"));
428 + let page = html(response);
429 + assert!(page.contains("notes.txt"));
430 + assert!(page.contains("5 B"));
431 + // Still offering the field, so a second file is one click away rather than
432 + // a reload.
433 + assert!(page.contains(r#"type="file""#));
434 + }
435 +
436 + #[tokio::test]
437 + async fn attaching_nothing_is_refused_on_the_field_it_came_from() {
438 + let state = state().await;
439 + let project = project(&state);
440 + let page = html(post(
441 + &state,
442 + &format!("/projects/{project}/attachments"),
443 + Params::new(),
444 + ));
445 +
446 + assert!(page.contains("Choose a file to attach."));
447 + assert!(page.contains("No attachments yet."));
448 + }
449 +
450 + #[tokio::test]
451 + async fn a_file_that_is_not_there_is_a_refusal_rather_than_a_failure() {
452 + // The user's to fix by picking another file, so it comes back on the field
453 + // instead of as a 500 that says nothing they can act on.
454 + let state = state().await;
455 + let project = project(&state);
456 + let page = html(attach(
457 + &state,
458 + project,
459 + std::path::Path::new("/nowhere/at/all.txt"),
460 + ));
461 +
462 + assert!(page.contains("File does not exist"));
463 + }
464 +
465 + #[tokio::test]
466 + async fn an_attachment_opens_as_a_file_address() {
467 + // The other half of the finding: opening is a one-way handoff, so it is a
468 + // redirect to somewhere this router does not answer rather than content.
469 + // The space in the name is the reason the address is percent-encoded — an
470 + // unescaped one truncates it at the first gap.
471 + let state = state().await;
472 + let project = project(&state);
473 + attach(&state, project, &a_file("field notes.txt", "hello"));
474 +
475 + let attachment = state
476 + .attachments
477 + .list_for_project(project, DESKTOP_USER_ID)
478 + .unwrap()
479 + .pop()
480 + .expect("the attach landed");
481 +
482 + let response = get(
483 + &state,
484 + &format!("/projects/{project}/attachments/{}/open", attachment.id),
485 + Params::new(),
486 + );
487 + let Outcome::Goto(action) = response.outcome else {
488 + panic!("opening hands the file over rather than answering with a screen");
489 + };
490 + let quasi_router::Destination::External(url) = action.destination else {
491 + panic!("a file lives outside anything this router answers");
492 + };
493 + assert!(url.starts_with("file:///"));
494 + assert!(url.ends_with("field%20notes.txt"));
495 + // And it is really there, under its own name rather than under its hash.
496 + let path = url.replace("file://", "").replace("%20", " ");
497 + assert_eq!(std::fs::read_to_string(path).unwrap(), "hello");
498 + }
499 +
500 + #[tokio::test]
501 + async fn opening_an_attachment_that_is_not_there_is_a_not_found() {
502 + let state = state().await;
503 + let project = project(&state);
504 + let error = router()
505 + .handle(
506 + &state,
507 + Method::Get,
508 + &format!("/projects/{project}/attachments/{}/open", uuid::Uuid::nil()),
509 + Params::new(),
510 + )
511 + .expect_err("no such attachment");
512 + assert_eq!(error.class.http_status(), 404);
398 513 }
399 514
400 515 #[tokio::test]