docs(05): create phase plan
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
f115082bd5
commit
5ce8b70f69
8 changed files with 878 additions and 11 deletions
|
|
@ -126,9 +126,16 @@ Plans:
|
||||||
|
|
||||||
**Plans:** 4 plans
|
**Plans:** 4 plans
|
||||||
Plans:
|
Plans:
|
||||||
|
**Wave 1**
|
||||||
- [ ] 05-01-PLAN.md — Wave 1: go get aws-sdk-go-v2 + migration 0005_files + sqlc queries + files.Store + FileStorer interface + RED test scaffold + MinIO in compose.yaml
|
- [ ] 05-01-PLAN.md — Wave 1: go get aws-sdk-go-v2 + migration 0005_files + sqlc queries + files.Store + FileStorer interface + RED test scaffold + MinIO in compose.yaml
|
||||||
|
|
||||||
|
**Wave 2** *(blocked on Wave 1 completion)*
|
||||||
- [ ] 05-02-PLAN.md — Wave 2: handlers_files.go (FilesDeps + FileUploadHandler + TabloFilesTabHandler + TabloTasksTabHandler) + tablos.templ 3-tab layout + files.templ (upload form + list) + router + main.go wiring (FILE-01, FILE-02, FILE-03, FILE-06)
|
- [ ] 05-02-PLAN.md — Wave 2: handlers_files.go (FilesDeps + FileUploadHandler + TabloFilesTabHandler + TabloTasksTabHandler) + tablos.templ 3-tab layout + files.templ (upload form + list) + router + main.go wiring (FILE-01, FILE-02, FILE-03, FILE-06)
|
||||||
|
|
||||||
|
**Wave 3** *(blocked on Wave 2 completion)*
|
||||||
- [ ] 05-03-PLAN.md — Wave 3: FileDownloadHandler (302 → presigned URL) + FileDeleteConfirmHandler + FileDeleteHandler (S3-first delete) + FileDeleteConfirmFragment + full TestFile* green (FILE-04, FILE-05, FILE-06)
|
- [ ] 05-03-PLAN.md — Wave 3: FileDownloadHandler (302 → presigned URL) + FileDeleteConfirmHandler + FileDeleteHandler (S3-first delete) + FileDeleteConfirmFragment + full TestFile* green (FILE-04, FILE-05, FILE-06)
|
||||||
|
|
||||||
|
**Wave 4** *(blocked on Wave 3 completion)*
|
||||||
- [ ] 05-04-PLAN.md — Wave 4: Human-verify checkpoint: tab navigation + upload/list/download/delete end-to-end browser walkthrough
|
- [ ] 05-04-PLAN.md — Wave 4: Human-verify checkpoint: tab navigation + upload/list/download/delete end-to-end browser walkthrough
|
||||||
|
|
||||||
### Phase 6: Background Worker
|
### Phase 6: Background Worker
|
||||||
|
|
|
||||||
|
|
@ -3,13 +3,13 @@ gsd_state_version: 1.0
|
||||||
milestone: v1.0
|
milestone: v1.0
|
||||||
milestone_name: milestone
|
milestone_name: milestone
|
||||||
status: ready_to_plan
|
status: ready_to_plan
|
||||||
last_updated: "2026-05-15T08:58:24.500Z"
|
last_updated: "2026-05-15T10:14:34.258Z"
|
||||||
progress:
|
progress:
|
||||||
total_phases: 7
|
total_phases: 7
|
||||||
completed_phases: 4
|
completed_phases: 4
|
||||||
total_plans: 18
|
total_plans: 22
|
||||||
completed_plans: 18
|
completed_plans: 18
|
||||||
percent: 100
|
percent: 82
|
||||||
---
|
---
|
||||||
|
|
||||||
# STATE
|
# STATE
|
||||||
|
|
|
||||||
|
|
@ -31,6 +31,8 @@ must_haves:
|
||||||
- "files.Store has Upload, Delete, PresignDownload methods satisfying FileStorer interface"
|
- "files.Store has Upload, Delete, PresignDownload methods satisfying FileStorer interface"
|
||||||
- "handlers_files_test.go contains RED test stubs for FILE-01..FILE-06 (compile but skip)"
|
- "handlers_files_test.go contains RED test stubs for FILE-01..FILE-06 (compile but skip)"
|
||||||
- "MinIO service runs in compose.yaml with mc init container creating xtablo-dev bucket"
|
- "MinIO service runs in compose.yaml with mc init container creating xtablo-dev bucket"
|
||||||
|
- "D-03: compose.yaml contains minio and minio-init services; minio-init creates xtablo-dev bucket with restart: no"
|
||||||
|
- "D-05: files.Store.Upload calls http.DetectContentType on first 512 bytes to detect content-type server-side"
|
||||||
artifacts:
|
artifacts:
|
||||||
- path: "backend/migrations/0005_files.sql"
|
- path: "backend/migrations/0005_files.sql"
|
||||||
provides: "tablo_files schema"
|
provides: "tablo_files schema"
|
||||||
|
|
|
||||||
|
|
@ -27,6 +27,14 @@ must_haves:
|
||||||
- "Only the tablo owner can see or POST to the files tab (non-owner gets 404)"
|
- "Only the tablo owner can see or POST to the files tab (non-owner gets 404)"
|
||||||
- "Tab bar on tablo detail page shows Overview / Tasks / Files with hx-push-url; URL updates on tab switch"
|
- "Tab bar on tablo detail page shows Overview / Tasks / Files with hx-push-url; URL updates on tab switch"
|
||||||
- "Uploading a file >25MB returns a friendly error message above the upload form (not a 500)"
|
- "Uploading a file >25MB returns a friendly error message above the upload form (not a 500)"
|
||||||
|
- "D-01: FileUploadHandler receives multipart POST and streams file body directly to S3 via files.Store.Upload (server-proxied, no temp file)"
|
||||||
|
- "D-02: main.go reads S3_ENDPOINT, S3_BUCKET, S3_ACCESS_KEY, S3_SECRET_KEY, S3_REGION, S3_USE_PATH_STYLE env vars and passes them to files.NewStore"
|
||||||
|
- "D-04: FileUploadHandler constructs S3 key as files/{tablo_id}/{uuid} before calling files.Store.Upload"
|
||||||
|
- "D-06: FileUploadHandler generates a new UUID per upload regardless of filename; no dedup check; duplicate filenames each get their own DB row"
|
||||||
|
- "D-07: TabloDetailPage signature includes activeTab string param; tab bar renders Overview, Tasks, Files links"
|
||||||
|
- "D-08: Tab bar links carry hx-push-url attribute matching their href so browser URL updates on HTMX tab switch"
|
||||||
|
- "D-09: FileUploadHandler sets r.Body = http.MaxBytesReader(w, r.Body, maxBytes) before ParseMultipartForm; max size read from MAX_UPLOAD_SIZE_MB env var (default 25)"
|
||||||
|
- "D-10: FileUploadHandler has no file-type allowlist check; all MIME types accepted after http.DetectContentType detection"
|
||||||
artifacts:
|
artifacts:
|
||||||
- path: "backend/internal/web/handlers_files.go"
|
- path: "backend/internal/web/handlers_files.go"
|
||||||
provides: "FilesDeps, TabloFilesTabHandler, FileUploadHandler, TabloTasksTabHandler"
|
provides: "FilesDeps, TabloFilesTabHandler, FileUploadHandler, TabloTasksTabHandler"
|
||||||
|
|
@ -148,6 +156,7 @@ From backend/internal/db/sqlc/files.sql.go (generated in Plan 01 — key types):
|
||||||
loadOwnedTabloForFile helper — same shape as loadOwnedTabloForTask but for file_id URL param and GetTabloFileByID query. Signature: func loadOwnedTabloForFile(w, r, deps FilesDeps) (sqlc.Tablo, sqlc.TabloFile, *auth.User, bool).
|
loadOwnedTabloForFile helper — same shape as loadOwnedTabloForTask but for file_id URL param and GetTabloFileByID query. Signature: func loadOwnedTabloForFile(w, r, deps FilesDeps) (sqlc.Tablo, sqlc.TabloFile, *auth.User, bool).
|
||||||
|
|
||||||
TabloFilesTabHandler (GET /tablos/{id}/files):
|
TabloFilesTabHandler (GET /tablos/{id}/files):
|
||||||
|
0. if deps.Files == nil { http.Error(w, "storage not configured", http.StatusServiceUnavailable); return } — MUST be the very first statement before any other logic
|
||||||
1. loadOwnedTablo → 404 on failure
|
1. loadOwnedTablo → 404 on failure
|
||||||
2. deps.Queries.ListFilesByTablo → log error + empty slice on failure
|
2. deps.Queries.ListFilesByTablo → log error + empty slice on failure
|
||||||
3. Set Content-Type: text/html; charset=utf-8
|
3. Set Content-Type: text/html; charset=utf-8
|
||||||
|
|
@ -159,12 +168,13 @@ From backend/internal/db/sqlc/files.sql.go (generated in Plan 01 — key types):
|
||||||
2. deps.Queries.ListTasksByTablo → log error + empty slice on failure
|
2. deps.Queries.ListTasksByTablo → log error + empty slice on failure
|
||||||
3. If HX-Request == "true": render templates.TasksTabFragment(tablo, tasks, csrf.Token(r)) — this component is created in Task 2 of this plan
|
3. If HX-Request == "true": render templates.TasksTabFragment(tablo, tasks, csrf.Token(r)) — this component is created in Task 2 of this plan
|
||||||
4. Else: render templates.TabloDetailPage(user, csrf.Token(r), tablo, tasks, nil, "tasks")
|
4. Else: render templates.TabloDetailPage(user, csrf.Token(r), tablo, tasks, nil, "tasks")
|
||||||
Note: TabloTasksTabHandler lives in handlers_files.go since it is part of the tab wiring introduced this phase. It takes FilesDeps (which has Queries). Alternatively put it in handlers_tasks.go — choose whichever file keeps imports clean.
|
File placement: TabloTasksTabHandler lives in handlers_files.go (it is part of the tab wiring introduced this phase). TasksTabFragment (the templ component wrapping KanbanBoard) goes in tablos.templ (it is a tablo-level concern). TabloTasksTabHandler does NOT go in handlers_tasks.go.
|
||||||
|
|
||||||
FileUploadHandler (POST /tablos/{id}/files):
|
FileUploadHandler (POST /tablos/{id}/files):
|
||||||
|
0. if deps.Files == nil { http.Error(w, "storage not configured", http.StatusServiceUnavailable); return } — MUST be the very first statement before reading the request body
|
||||||
1. loadOwnedTablo → 404 on failure
|
1. loadOwnedTablo → 404 on failure
|
||||||
2. maxBytes := int64(deps.MaxUploadMB) * 1024 * 1024
|
2. maxBytes := int64(deps.MaxUploadMB) * 1024 * 1024
|
||||||
3. r.Body = http.MaxBytesReader(w, r.Body, maxBytes) — MUST be first, before ParseMultipartForm (Pitfall 2 in RESEARCH)
|
3. r.Body = http.MaxBytesReader(w, r.Body, maxBytes) — MUST be before ParseMultipartForm (Pitfall 2 in RESEARCH)
|
||||||
4. if err := r.ParseMultipartForm(2 << 20); err != nil: check errors.As(err, &mbErr) for *http.MaxBytesError; if yes, render upload error fragment with "File too large (max {MaxUploadMB} MB)." and return 422; else http.Error 400
|
4. if err := r.ParseMultipartForm(2 << 20); err != nil: check errors.As(err, &mbErr) for *http.MaxBytesError; if yes, render upload error fragment with "File too large (max {MaxUploadMB} MB)." and return 422; else http.Error 400
|
||||||
5. file, header, err := r.FormFile("file") — 400 on error
|
5. file, header, err := r.FormFile("file") — 400 on error
|
||||||
6. defer file.Close()
|
6. defer file.Close()
|
||||||
|
|
@ -187,7 +197,9 @@ From backend/internal/db/sqlc/files.sql.go (generated in Plan 01 — key types):
|
||||||
<acceptance_criteria>
|
<acceptance_criteria>
|
||||||
- go build ./... exits 0
|
- go build ./... exits 0
|
||||||
- backend/internal/web/handlers_files.go contains "type FilesDeps struct" and "func TabloFilesTabHandler(" and "func FileUploadHandler(" and "func TabloTasksTabHandler("
|
- backend/internal/web/handlers_files.go contains "type FilesDeps struct" and "func TabloFilesTabHandler(" and "func FileUploadHandler(" and "func TabloTasksTabHandler("
|
||||||
- FileUploadHandler contains "http.MaxBytesReader" on the line before "ParseMultipartForm" (Pitfall 2 guard)
|
- FileUploadHandler first statement is "deps.Files == nil" nil guard returning 503 before any body read
|
||||||
|
- TabloFilesTabHandler first statement is "deps.Files == nil" nil guard returning 503
|
||||||
|
- FileUploadHandler contains "http.MaxBytesReader" before "ParseMultipartForm" (Pitfall 2 guard)
|
||||||
- FileUploadHandler contains "files/" string for S3 key construction (D-04)
|
- FileUploadHandler contains "files/" string for S3 key construction (D-04)
|
||||||
- FileUploadHandler contains "http.MaxBytesError" for size violation detection
|
- FileUploadHandler contains "http.MaxBytesError" for size violation detection
|
||||||
- go test ./internal/web/ -run "TestFileUpload|TestFilesTab" exits 0 (PASS or SKIP, no FAIL)
|
- go test ./internal/web/ -run "TestFileUpload|TestFilesTab" exits 0 (PASS or SKIP, no FAIL)
|
||||||
|
|
@ -234,7 +246,7 @@ From backend/internal/db/sqlc/files.sql.go (generated in Plan 01 — key types):
|
||||||
|
|
||||||
Also add stand-alone fragment components that can be returned by HTMX tab-switch responses:
|
Also add stand-alone fragment components that can be returned by HTMX tab-switch responses:
|
||||||
templ TabloOverviewTabFragment(tablo sqlc.Tablo, csrfToken string) — minimal overview content
|
templ TabloOverviewTabFragment(tablo sqlc.Tablo, csrfToken string) — minimal overview content
|
||||||
templ TasksTabFragment(tablo sqlc.Tablo, tasks []sqlc.Task, csrfToken string) — wraps existing @KanbanBoard(tablo.ID, csrfToken, tasks) call
|
templ TasksTabFragment(tablo sqlc.Tablo, tasks []sqlc.Task, csrfToken string) — wraps existing @KanbanBoard(tablo.ID, csrfToken, tasks) call; lives in tablos.templ (tablo-level concern)
|
||||||
These are called by TabloTasksTabHandler when HX-Request == "true" (Plan 02 Task 1).
|
These are called by TabloTasksTabHandler when HX-Request == "true" (Plan 02 Task 1).
|
||||||
|
|
||||||
Step 2 — Update TWO call sites in backend/internal/web/handlers_tablos.go:
|
Step 2 — Update TWO call sites in backend/internal/web/handlers_tablos.go:
|
||||||
|
|
@ -274,7 +286,7 @@ From backend/internal/db/sqlc/files.sql.go (generated in Plan 01 — key types):
|
||||||
maxUploadMB int: parse MAX_UPLOAD_SIZE_MB env var with strconv.Atoi, default 25 on empty/error
|
maxUploadMB int: parse MAX_UPLOAD_SIZE_MB env var with strconv.Atoi, default 25 on empty/error
|
||||||
If s3Endpoint == "" || s3Bucket == "": log slog.Warn (not Error + Exit — allow server to start without S3 for non-file routes in dev)
|
If s3Endpoint == "" || s3Bucket == "": log slog.Warn (not Error + Exit — allow server to start without S3 for non-file routes in dev)
|
||||||
filesStore, err := files.NewStore(ctx, s3Endpoint, s3Bucket, s3Region, s3AccessKey, s3SecretKey, s3UsePathStyle) — if err != nil: log slog.Error + os.Exit(1)
|
filesStore, err := files.NewStore(ctx, s3Endpoint, s3Bucket, s3Region, s3AccessKey, s3SecretKey, s3UsePathStyle) — if err != nil: log slog.Error + os.Exit(1)
|
||||||
Only construct filesStore if s3Endpoint != "" — else set filesStore = nil and fileDeps.Files = nil. File handlers must check for nil Files and return 503 "storage not configured".
|
Only construct filesStore if s3Endpoint != "" — else set filesStore = nil and fileDeps.Files = nil. File handlers check for nil Files as their FIRST statement and return 503 "storage not configured".
|
||||||
fileDeps := web.FilesDeps{Queries: q, Files: filesStore, MaxUploadMB: maxUploadMB}
|
fileDeps := web.FilesDeps{Queries: q, Files: filesStore, MaxUploadMB: maxUploadMB}
|
||||||
Update NewRouter call to pass fileDeps before csrfKey: web.NewRouter(pool, "./static", deps, tabloDeps, taskDeps, fileDeps, csrfKey, env)
|
Update NewRouter call to pass fileDeps before csrfKey: web.NewRouter(pool, "./static", deps, tabloDeps, taskDeps, fileDeps, csrfKey, env)
|
||||||
|
|
||||||
|
|
@ -290,6 +302,7 @@ From backend/internal/db/sqlc/files.sql.go (generated in Plan 01 — key types):
|
||||||
- go build ./... exits 0
|
- go build ./... exits 0
|
||||||
- go test ./... exits 0 (no regressions in existing test suite; TestTask* and TestTablo* remain PASS)
|
- go test ./... exits 0 (no regressions in existing test suite; TestTask* and TestTablo* remain PASS)
|
||||||
- backend/templates/tablos.templ contains "activeTab string" in TabloDetailPage signature and "hx-push-url" and "tab-content"
|
- backend/templates/tablos.templ contains "activeTab string" in TabloDetailPage signature and "hx-push-url" and "tab-content"
|
||||||
|
- backend/templates/tablos.templ contains "TasksTabFragment" component (wrapping KanbanBoard)
|
||||||
- backend/templates/files.templ contains "FilesTabFragment" and "FileUploadForm" and "hx-encoding" (required for HTMX multipart) and "FileRowGone"
|
- backend/templates/files.templ contains "FilesTabFragment" and "FileUploadForm" and "hx-encoding" (required for HTMX multipart) and "FileRowGone"
|
||||||
- backend/internal/web/router.go contains "fileDeps FilesDeps" in NewRouter signature and "TabloFilesTabHandler" and "TabloTasksTabHandler"
|
- backend/internal/web/router.go contains "fileDeps FilesDeps" in NewRouter signature and "TabloFilesTabHandler" and "TabloTasksTabHandler"
|
||||||
- backend/cmd/web/main.go contains "files.NewStore" and "fileDeps := web.FilesDeps"
|
- backend/cmd/web/main.go contains "files.NewStore" and "fileDeps := web.FilesDeps"
|
||||||
|
|
@ -305,7 +318,7 @@ From backend/internal/db/sqlc/files.sql.go (generated in Plan 01 — key types):
|
||||||
| Boundary | Description |
|
| Boundary | Description |
|
||||||
|----------|-------------|
|
|----------|-------------|
|
||||||
| browser → FileUploadHandler | Multipart form body; attacker controls filename, file content, MIME hints, and Content-Length |
|
| browser → FileUploadHandler | Multipart form body; attacker controls filename, file content, MIME hints, and Content-Length |
|
||||||
| FilesDeps.Files (FileStorer) | Nil-checked in main.go; handlers must check for nil before calling Upload/Delete/PresignDownload |
|
| FilesDeps.Files (FileStorer) | Nil-checked as the first statement of every file handler; returns 503 before touching the request body |
|
||||||
|
|
||||||
## STRIDE Threat Register
|
## STRIDE Threat Register
|
||||||
|
|
||||||
|
|
@ -316,7 +329,7 @@ From backend/internal/db/sqlc/files.sql.go (generated in Plan 01 — key types):
|
||||||
| T-05-02-03 | Spoofing | FileUploadHandler — content-type | mitigate | Browser Content-Type header ignored; server calls http.DetectContentType on first 512 bytes via files.Store.Upload (D-05) |
|
| T-05-02-03 | Spoofing | FileUploadHandler — content-type | mitigate | Browser Content-Type header ignored; server calls http.DetectContentType on first 512 bytes via files.Store.Upload (D-05) |
|
||||||
| T-05-02-04 | Elevation of Privilege | TabloFilesTabHandler — IDOR | mitigate | loadOwnedTablo called as first step in every handler; GetTabloByID query includes UserID filter; non-owner gets 404 (FILE-06) |
|
| T-05-02-04 | Elevation of Privilege | TabloFilesTabHandler — IDOR | mitigate | loadOwnedTablo called as first step in every handler; GetTabloByID query includes UserID filter; non-owner gets 404 (FILE-06) |
|
||||||
| T-05-02-05 | Tampering | File routes — CSRF | mitigate | gorilla/csrf middleware already in stack (Phase 2); all state-changing POSTs require valid CSRF token; @ui.CSRFField(csrfToken) present in upload form and future delete form |
|
| T-05-02-05 | Tampering | File routes — CSRF | mitigate | gorilla/csrf middleware already in stack (Phase 2); all state-changing POSTs require valid CSRF token; @ui.CSRFField(csrfToken) present in upload form and future delete form |
|
||||||
| T-05-02-06 | Denial of Service | TabloFilesTabHandler — nil FileStorer | mitigate | main.go only constructs filesStore when S3_ENDPOINT set; FileUploadHandler checks deps.Files == nil and returns 503 "storage not configured" before reading body |
|
| T-05-02-06 | Denial of Service | TabloFilesTabHandler + FileUploadHandler — nil FileStorer | mitigate | deps.Files == nil guard is the FIRST statement in TabloFilesTabHandler and FileUploadHandler; returns 503 "storage not configured" before any body read or DB call; main.go sets fileDeps.Files = nil when S3_ENDPOINT is unset |
|
||||||
</threat_model>
|
</threat_model>
|
||||||
|
|
||||||
<verification>
|
<verification>
|
||||||
|
|
@ -326,6 +339,7 @@ After both tasks:
|
||||||
- cd backend && go test ./internal/web/ -run TestFileUpload -v shows PASS or SKIP (not FAIL)
|
- cd backend && go test ./internal/web/ -run TestFileUpload -v shows PASS or SKIP (not FAIL)
|
||||||
- backend/templates/files.templ exists and contains FilesTabFragment, FileUploadForm, FileRowGone
|
- backend/templates/files.templ exists and contains FilesTabFragment, FileUploadForm, FileRowGone
|
||||||
- backend/templates/tablos.templ TabloDetailPage signature has 6 args (including files []sqlc.TabloFile and activeTab string)
|
- backend/templates/tablos.templ TabloDetailPage signature has 6 args (including files []sqlc.TabloFile and activeTab string)
|
||||||
|
- backend/templates/tablos.templ contains TasksTabFragment component
|
||||||
- router.go NewRouter has fileDeps FilesDeps parameter
|
- router.go NewRouter has fileDeps FilesDeps parameter
|
||||||
</verification>
|
</verification>
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -148,16 +148,19 @@ From backend/internal/web/handlers_tasks.go (delete handler pattern lines 285-30
|
||||||
</behavior>
|
</behavior>
|
||||||
<action>
|
<action>
|
||||||
Replace FileDownloadHandler stub (currently returns 501) with full implementation:
|
Replace FileDownloadHandler stub (currently returns 501) with full implementation:
|
||||||
|
0. if deps.Files == nil { http.Error(w, "storage not configured", http.StatusServiceUnavailable); return } — MUST be the very first statement
|
||||||
1. tablo, file, _, ok := loadOwnedTabloForFile(w, r, deps) — 404 on failure (FILE-06)
|
1. tablo, file, _, ok := loadOwnedTabloForFile(w, r, deps) — 404 on failure (FILE-06)
|
||||||
2. url, err := deps.Files.PresignDownload(r.Context(), file.S3Key) — 500 on error; log with slog.Default().Error("files download: PresignDownload failed", "file_id", file.ID, "err", err)
|
2. url, err := deps.Files.PresignDownload(r.Context(), file.S3Key) — 500 on error; log with slog.Default().Error("files download: PresignDownload failed", "file_id", file.ID, "err", err)
|
||||||
3. http.Redirect(w, r, url, http.StatusFound) — 302 redirect to signed URL (per CONTEXT.md discretion item: download redirects to signed URL)
|
3. http.Redirect(w, r, url, http.StatusFound) — 302 redirect to signed URL (per CONTEXT.md discretion item: download redirects to signed URL)
|
||||||
|
|
||||||
Replace FileDeleteConfirmHandler stub with implementation (inline confirm pattern from Phase 3):
|
Replace FileDeleteConfirmHandler stub with implementation (inline confirm pattern from Phase 3):
|
||||||
|
0. if deps.Files == nil { http.Error(w, "storage not configured", http.StatusServiceUnavailable); return } — MUST be the very first statement
|
||||||
1. tablo, file, _, ok := loadOwnedTabloForFile(w, r, deps) — 404 on failure
|
1. tablo, file, _, ok := loadOwnedTabloForFile(w, r, deps) — 404 on failure
|
||||||
2. Set Content-Type: text/html; charset=utf-8
|
2. Set Content-Type: text/html; charset=utf-8
|
||||||
3. Render templates.FileDeleteConfirmFragment(tablo.ID, file, csrf.Token(r)) — new component created in Task 2
|
3. Render templates.FileDeleteConfirmFragment(tablo.ID, file, csrf.Token(r)) — new component created in Task 2
|
||||||
|
|
||||||
Replace FileDeleteHandler stub with implementation (S3-first, always-delete-DB per RESEARCH.md Pattern 6):
|
Replace FileDeleteHandler stub with implementation (S3-first, always-delete-DB per RESEARCH.md Pattern 6):
|
||||||
|
0. if deps.Files == nil { http.Error(w, "storage not configured", http.StatusServiceUnavailable); return } — MUST be the very first statement
|
||||||
1. tablo, file, _, ok := loadOwnedTabloForFile(w, r, deps) — 404 on failure
|
1. tablo, file, _, ok := loadOwnedTabloForFile(w, r, deps) — 404 on failure
|
||||||
2. Delete from S3: if err := deps.Files.Delete(r.Context(), file.S3Key); err != nil: slog.Default().Error("files delete: S3 Delete failed", "key", file.S3Key, "err", err) — log but do NOT return; continue to DB delete (orphan S3 objects are Phase 6 worker's problem per CONTEXT.md deferred items)
|
2. Delete from S3: if err := deps.Files.Delete(r.Context(), file.S3Key); err != nil: slog.Default().Error("files delete: S3 Delete failed", "key", file.S3Key, "err", err) — log but do NOT return; continue to DB delete (orphan S3 objects are Phase 6 worker's problem per CONTEXT.md deferred items)
|
||||||
3. if err := deps.Queries.DeleteTabloFile(r.Context(), sqlc.DeleteTabloFileParams{ID: file.ID, TabloID: tablo.ID}); err != nil: log + http.Error 500 + return
|
3. if err := deps.Queries.DeleteTabloFile(r.Context(), sqlc.DeleteTabloFileParams{ID: file.ID, TabloID: tablo.ID}); err != nil: log + http.Error 500 + return
|
||||||
|
|
@ -174,6 +177,9 @@ From backend/internal/web/handlers_tasks.go (delete handler pattern lines 285-30
|
||||||
</done>
|
</done>
|
||||||
<acceptance_criteria>
|
<acceptance_criteria>
|
||||||
- go test ./internal/web/ -run "TestFileDownload|TestFileDelete|TestFileOwnership" exits 0 with "--- PASS" for each test (no SKIP, no FAIL)
|
- go test ./internal/web/ -run "TestFileDownload|TestFileDelete|TestFileOwnership" exits 0 with "--- PASS" for each test (no SKIP, no FAIL)
|
||||||
|
- backend/internal/web/handlers_files.go FileDownloadHandler first statement is "deps.Files == nil" nil guard returning 503
|
||||||
|
- backend/internal/web/handlers_files.go FileDeleteConfirmHandler first statement is "deps.Files == nil" nil guard returning 503
|
||||||
|
- backend/internal/web/handlers_files.go FileDeleteHandler first statement is "deps.Files == nil" nil guard returning 503
|
||||||
- backend/internal/web/handlers_files.go FileDownloadHandler contains "http.StatusFound" (302 redirect)
|
- backend/internal/web/handlers_files.go FileDownloadHandler contains "http.StatusFound" (302 redirect)
|
||||||
- backend/internal/web/handlers_files.go FileDeleteHandler contains "deps.Files.Delete" on a line before "deps.Queries.DeleteTabloFile" (S3 first per RESEARCH Pattern 6)
|
- backend/internal/web/handlers_files.go FileDeleteHandler contains "deps.Files.Delete" on a line before "deps.Queries.DeleteTabloFile" (S3 first per RESEARCH Pattern 6)
|
||||||
- backend/internal/web/handlers_files.go FileDeleteHandler: the slog.Error for S3 failure does NOT have an early return; DB delete is always attempted
|
- backend/internal/web/handlers_files.go FileDeleteHandler: the slog.Error for S3 failure does NOT have an early return; DB delete is always attempted
|
||||||
|
|
@ -244,6 +250,7 @@ From backend/internal/web/handlers_tasks.go (delete handler pattern lines 285-30
|
||||||
| T-05-03-03 | Elevation of Privilege | FileDeleteHandler — IDOR | mitigate | Same loadOwnedTabloForFile preamble; DeleteTabloFile parameterized with both file ID AND tablo_id — non-owner cannot delete even with a valid file UUID |
|
| T-05-03-03 | Elevation of Privilege | FileDeleteHandler — IDOR | mitigate | Same loadOwnedTabloForFile preamble; DeleteTabloFile parameterized with both file ID AND tablo_id — non-owner cannot delete even with a valid file UUID |
|
||||||
| T-05-03-04 | Denial of Service | FileDeleteHandler — partial S3 failure loop | accept | S3 delete failure is logged but does not abort; DB row is always deleted; orphan objects accumulate and are cleaned by Phase 6 worker (explicit design decision from CONTEXT.md deferred items) |
|
| T-05-03-04 | Denial of Service | FileDeleteHandler — partial S3 failure loop | accept | S3 delete failure is logged but does not abort; DB row is always deleted; orphan objects accumulate and are cleaned by Phase 6 worker (explicit design decision from CONTEXT.md deferred items) |
|
||||||
| T-05-03-05 | Tampering | FileDeleteConfirmFragment — CSRF on delete form | mitigate | @ui.CSRFField(csrfToken) present in FileDeleteConfirmFragment form; gorilla/csrf middleware rejects POST /files/{id}/delete without valid token |
|
| T-05-03-05 | Tampering | FileDeleteConfirmFragment — CSRF on delete form | mitigate | @ui.CSRFField(csrfToken) present in FileDeleteConfirmFragment form; gorilla/csrf middleware rejects POST /files/{id}/delete without valid token |
|
||||||
|
| T-05-03-06 | Denial of Service | FileDownloadHandler + FileDeleteConfirmHandler + FileDeleteHandler — nil FileStorer | mitigate | deps.Files == nil guard is the FIRST statement in all three handlers; returns 503 "storage not configured" before any S3 or DB call |
|
||||||
</threat_model>
|
</threat_model>
|
||||||
|
|
||||||
<verification>
|
<verification>
|
||||||
|
|
@ -251,6 +258,7 @@ After both tasks:
|
||||||
- cd backend && go test ./... -count=1 -timeout 60s exits 0 (all TestFile* PASS, all TestTask* and TestTablo* still PASS)
|
- cd backend && go test ./... -count=1 -timeout 60s exits 0 (all TestFile* PASS, all TestTask* and TestTablo* still PASS)
|
||||||
- cd backend && go test ./internal/web/ -run TestFile -v shows 6x "--- PASS"
|
- cd backend && go test ./internal/web/ -run TestFile -v shows 6x "--- PASS"
|
||||||
- backend/internal/web/handlers_files.go contains no "501" (all stubs replaced)
|
- backend/internal/web/handlers_files.go contains no "501" (all stubs replaced)
|
||||||
|
- backend/internal/web/handlers_files.go FileDownloadHandler, FileDeleteConfirmHandler, FileDeleteHandler each have "deps.Files == nil" as the first guard
|
||||||
- backend/templates/files.templ contains FileDeleteConfirmFragment with ui.CSRFField
|
- backend/templates/files.templ contains FileDeleteConfirmFragment with ui.CSRFField
|
||||||
</verification>
|
</verification>
|
||||||
|
|
||||||
|
|
|
||||||
750
.planning/phases/05-files/05-PATTERNS.md
Normal file
750
.planning/phases/05-files/05-PATTERNS.md
Normal file
|
|
@ -0,0 +1,750 @@
|
||||||
|
# Phase 5: Files - Pattern Map
|
||||||
|
|
||||||
|
**Mapped:** 2026-05-15
|
||||||
|
**Files analyzed:** 9 (new/modified)
|
||||||
|
**Analogs found:** 9 / 9
|
||||||
|
|
||||||
|
## File Classification
|
||||||
|
|
||||||
|
| New/Modified File | Role | Data Flow | Closest Analog | Match Quality |
|
||||||
|
|-------------------|------|-----------|----------------|---------------|
|
||||||
|
| `backend/migrations/0005_files.sql` | migration | batch | `backend/migrations/0004_tasks.sql` | exact |
|
||||||
|
| `backend/internal/db/queries/files.sql` | query | CRUD | `backend/internal/db/queries/tasks.sql` | exact |
|
||||||
|
| `backend/internal/files/store.go` | service | file-I/O | `backend/internal/files/doc.go` (placeholder) | new (no analog body) |
|
||||||
|
| `backend/internal/web/handlers_files.go` | controller | request-response | `backend/internal/web/handlers_tasks.go` | exact |
|
||||||
|
| `backend/internal/web/router.go` | config | request-response | `backend/internal/web/router.go` (self, modify) | exact |
|
||||||
|
| `backend/templates/tablos/detail.templ` (= `backend/templates/tablos.templ`) | component | request-response | `backend/templates/tablos.templ` (self, modify) | exact |
|
||||||
|
| `backend/templates/files.templ` | component | request-response | `backend/templates/tasks.templ` | role-match |
|
||||||
|
| `backend/cmd/web/main.go` | config | request-response | `backend/cmd/web/main.go` (self, modify) | exact |
|
||||||
|
| `compose.yaml` | config | — | `backend/compose.yaml` (self, modify) | exact |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Pattern Assignments
|
||||||
|
|
||||||
|
### `backend/migrations/0005_files.sql` (migration, batch)
|
||||||
|
|
||||||
|
**Analog:** `backend/migrations/0004_tasks.sql`
|
||||||
|
|
||||||
|
**Full migration structure** (lines 1–26):
|
||||||
|
```sql
|
||||||
|
-- migrations/0004_tasks.sql
|
||||||
|
-- Phase 4: Tasks (Kanban)
|
||||||
|
|
||||||
|
-- +goose Up
|
||||||
|
|
||||||
|
-- ENUM declaration order must match visual left-to-right column order (Pitfall 6).
|
||||||
|
CREATE TYPE task_status AS ENUM ('todo', 'in_progress', 'in_review', 'done');
|
||||||
|
|
||||||
|
CREATE TABLE tasks (
|
||||||
|
id uuid PRIMARY KEY DEFAULT gen_random_uuid(),
|
||||||
|
tablo_id uuid NOT NULL REFERENCES tablos(id) ON DELETE CASCADE,
|
||||||
|
title text NOT NULL,
|
||||||
|
description text,
|
||||||
|
status task_status NOT NULL DEFAULT 'todo',
|
||||||
|
position integer NOT NULL DEFAULT 100,
|
||||||
|
created_at timestamptz NOT NULL DEFAULT now(),
|
||||||
|
updated_at timestamptz NOT NULL DEFAULT now()
|
||||||
|
);
|
||||||
|
|
||||||
|
CREATE INDEX tasks_tablo_id_status_idx ON tasks(tablo_id, status, position);
|
||||||
|
|
||||||
|
-- +goose Down
|
||||||
|
-- Table MUST be dropped before type (Pitfall 3 — type is still referenced by table column).
|
||||||
|
DROP TABLE IF EXISTS tasks;
|
||||||
|
DROP TYPE IF EXISTS task_status;
|
||||||
|
```
|
||||||
|
|
||||||
|
**Key conventions to copy:**
|
||||||
|
- Header comment: `-- migrations/0005_files.sql` + phase description line
|
||||||
|
- `-- +goose Up` / `-- +goose Down` section markers (required by goose)
|
||||||
|
- `id uuid PRIMARY KEY DEFAULT gen_random_uuid()` for all new tables
|
||||||
|
- `tablo_id uuid NOT NULL REFERENCES tablos(id) ON DELETE CASCADE` — CASCADE is the pattern; file rows are cleaned when tablo is deleted
|
||||||
|
- `timestamptz NOT NULL DEFAULT now()` — always timestamptz, never timestamp
|
||||||
|
- Index comment explains composite key rationale
|
||||||
|
- `DROP TABLE IF EXISTS` in Down block (no DROP TYPE needed since `tablo_files` has no custom ENUM)
|
||||||
|
|
||||||
|
**Deviation for 0005:** No `updated_at` column (files are immutable — upload-once, delete-only). Use `bigint` for `size_bytes` (not `integer`). No custom ENUM type.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### `backend/internal/db/queries/files.sql` (query, CRUD)
|
||||||
|
|
||||||
|
**Analog:** `backend/internal/db/queries/tasks.sql`
|
||||||
|
|
||||||
|
**Full query file** (lines 1–30):
|
||||||
|
```sql
|
||||||
|
-- name: ListTasksByTablo :many
|
||||||
|
SELECT id, tablo_id, title, description, status, position, created_at, updated_at
|
||||||
|
FROM tasks
|
||||||
|
WHERE tablo_id = $1
|
||||||
|
ORDER BY status, position, created_at;
|
||||||
|
|
||||||
|
-- name: InsertTask :one
|
||||||
|
INSERT INTO tasks (tablo_id, title, description, status, position)
|
||||||
|
VALUES ($1, $2, $3, $4, $5)
|
||||||
|
RETURNING id, tablo_id, title, description, status, position, created_at, updated_at;
|
||||||
|
|
||||||
|
-- name: GetTaskByID :one
|
||||||
|
SELECT id, tablo_id, title, description, status, position, created_at, updated_at
|
||||||
|
FROM tasks
|
||||||
|
WHERE id = $1 AND tablo_id = $2;
|
||||||
|
|
||||||
|
-- name: DeleteTask :exec
|
||||||
|
DELETE FROM tasks WHERE id = $1 AND tablo_id = $2;
|
||||||
|
```
|
||||||
|
|
||||||
|
**Conventions to copy:**
|
||||||
|
- `-- name: QueryName :return_type` annotation on every query (required by sqlc)
|
||||||
|
- `:one` for INSERT RETURNING + single SELECT; `:many` for list; `:exec` for DELETE
|
||||||
|
- Always include `tablo_id = $N` in WHERE clauses for ownership (IDOR prevention)
|
||||||
|
- RETURNING clause lists all columns explicitly (not `RETURNING *`)
|
||||||
|
- ORDER BY in list queries: `created_at DESC` (newest first) for files
|
||||||
|
|
||||||
|
**Files query set needed:** `InsertTabloFile :one`, `ListFilesByTablo :many`, `GetTabloFileByID :one`, `DeleteTabloFile :exec`
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### `backend/internal/files/store.go` (service, file-I/O)
|
||||||
|
|
||||||
|
**Analog:** No body analog in codebase (placeholder only at `backend/internal/files/doc.go`). Use RESEARCH.md Patterns 1–3 exclusively.
|
||||||
|
|
||||||
|
**Package declaration** (`doc.go` line 1–2):
|
||||||
|
```go
|
||||||
|
// Package files is a Phase 1 placeholder; the upload/storage implementation lands in Phase 5.
|
||||||
|
package files
|
||||||
|
```
|
||||||
|
|
||||||
|
**Implementation pattern** from RESEARCH.md (Patterns 1–3 — verified against aws-sdk-go-v2 docs):
|
||||||
|
|
||||||
|
Store struct and constructor:
|
||||||
|
```go
|
||||||
|
type Store struct {
|
||||||
|
client *s3.Client
|
||||||
|
bucket string
|
||||||
|
}
|
||||||
|
|
||||||
|
func NewStore(ctx context.Context, endpoint, bucket, region, accessKey, secretKey string, usePathStyle bool) (*Store, error) {
|
||||||
|
cfg, err := config.LoadDefaultConfig(ctx,
|
||||||
|
config.WithRegion(region),
|
||||||
|
config.WithCredentialsProvider(
|
||||||
|
credentials.NewStaticCredentialsProvider(accessKey, secretKey, ""),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
client := s3.NewFromConfig(cfg, func(o *s3.Options) {
|
||||||
|
o.BaseEndpoint = aws.String(endpoint)
|
||||||
|
o.UsePathStyle = usePathStyle // true for MinIO, false or omit for R2
|
||||||
|
})
|
||||||
|
return &Store{client: client, bucket: bucket}, nil
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Upload (sniff + stream pattern):
|
||||||
|
```go
|
||||||
|
func (s *Store) Upload(ctx context.Context, key string, file io.Reader) (contentType string, bytesWritten int64, err error) {
|
||||||
|
var sniffBuf [512]byte
|
||||||
|
n, readErr := io.ReadFull(file, sniffBuf[:])
|
||||||
|
// Accept io.ErrUnexpectedEOF — normal for files < 512 bytes (Pitfall 3)
|
||||||
|
if readErr != nil && !errors.Is(readErr, io.ErrUnexpectedEOF) {
|
||||||
|
return "", 0, readErr
|
||||||
|
}
|
||||||
|
contentType = http.DetectContentType(sniffBuf[:n])
|
||||||
|
body := io.MultiReader(bytes.NewReader(sniffBuf[:n]), file)
|
||||||
|
// Wrap body in a counting reader before passing to PutObject (Pitfall 8)
|
||||||
|
// ...
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
PresignDownload:
|
||||||
|
```go
|
||||||
|
func (s *Store) PresignDownload(ctx context.Context, key string) (string, error) {
|
||||||
|
presignClient := s3.NewPresignClient(s.client)
|
||||||
|
req, err := presignClient.PresignGetObject(ctx, &s3.GetObjectInput{
|
||||||
|
Bucket: aws.String(s.bucket),
|
||||||
|
Key: aws.String(key),
|
||||||
|
}, func(o *s3.PresignOptions) {
|
||||||
|
o.Expires = 5 * time.Minute
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
|
return req.URL, nil
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Delete:
|
||||||
|
```go
|
||||||
|
func (s *Store) Delete(ctx context.Context, key string) error {
|
||||||
|
_, err := s.client.DeleteObject(ctx, &s3.DeleteObjectInput{
|
||||||
|
Bucket: aws.String(s.bucket),
|
||||||
|
Key: aws.String(key),
|
||||||
|
})
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Interface for test injection** (per RESEARCH.md validation architecture):
|
||||||
|
```go
|
||||||
|
type FileStorer interface {
|
||||||
|
Upload(ctx context.Context, key string, file io.Reader) (contentType string, bytesWritten int64, err error)
|
||||||
|
Delete(ctx context.Context, key string) error
|
||||||
|
PresignDownload(ctx context.Context, key string) (string, error)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### `backend/internal/web/handlers_files.go` (controller, request-response)
|
||||||
|
|
||||||
|
**Analog:** `backend/internal/web/handlers_tasks.go`
|
||||||
|
|
||||||
|
**Imports pattern** (lines 1–19 of handlers_tasks.go):
|
||||||
|
```go
|
||||||
|
package web
|
||||||
|
|
||||||
|
import (
|
||||||
|
"errors"
|
||||||
|
"log/slog"
|
||||||
|
"net/http"
|
||||||
|
|
||||||
|
"backend/internal/auth"
|
||||||
|
"backend/internal/db/sqlc"
|
||||||
|
"backend/templates"
|
||||||
|
|
||||||
|
"github.com/go-chi/chi/v5"
|
||||||
|
"github.com/google/uuid"
|
||||||
|
"github.com/gorilla/csrf"
|
||||||
|
"github.com/jackc/pgx/v5"
|
||||||
|
)
|
||||||
|
```
|
||||||
|
|
||||||
|
**FilesDeps struct pattern** (mirrors TasksDeps, lines 21–24 of handlers_tasks.go):
|
||||||
|
```go
|
||||||
|
// TasksDeps holds dependencies for all task handlers.
|
||||||
|
type TasksDeps struct {
|
||||||
|
Queries *sqlc.Queries
|
||||||
|
}
|
||||||
|
```
|
||||||
|
Copy this exactly:
|
||||||
|
```go
|
||||||
|
// FilesDeps holds dependencies for all file handlers.
|
||||||
|
type FilesDeps struct {
|
||||||
|
Queries *sqlc.Queries
|
||||||
|
Files FileStorer // interface — allows stub injection in tests
|
||||||
|
MaxUploadMB int // parsed from MAX_UPLOAD_SIZE_MB env var (default 25)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**loadOwnedTabloForTask pattern** — copy for `loadOwnedTabloForFile` (lines 44–76):
|
||||||
|
```go
|
||||||
|
func loadOwnedTabloForTask(w http.ResponseWriter, r *http.Request, deps TasksDeps) (sqlc.Tablo, sqlc.Task, *auth.User, bool) {
|
||||||
|
tablo, user, ok := loadOwnedTablo(w, r, TablosDeps{Queries: deps.Queries})
|
||||||
|
if !ok {
|
||||||
|
return sqlc.Tablo{}, sqlc.Task{}, nil, false
|
||||||
|
}
|
||||||
|
taskID, err := uuid.Parse(chi.URLParam(r, "task_id"))
|
||||||
|
if err != nil {
|
||||||
|
http.NotFound(w, r)
|
||||||
|
return sqlc.Tablo{}, sqlc.Task{}, nil, false
|
||||||
|
}
|
||||||
|
task, err := deps.Queries.GetTaskByID(r.Context(), sqlc.GetTaskByIDParams{
|
||||||
|
ID: taskID,
|
||||||
|
TabloID: tablo.ID,
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
if errors.Is(err, pgx.ErrNoRows) {
|
||||||
|
http.NotFound(w, r)
|
||||||
|
return sqlc.Tablo{}, sqlc.Task{}, nil, false
|
||||||
|
}
|
||||||
|
slog.Default().Error("tasks: GetTaskByID failed", "id", taskID, "err", err)
|
||||||
|
http.Error(w, "internal server error", http.StatusInternalServerError)
|
||||||
|
return sqlc.Tablo{}, sqlc.Task{}, nil, false
|
||||||
|
}
|
||||||
|
return tablo, task, user, true
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Handler constructor pattern** — all handlers follow this shape (lines 80–98):
|
||||||
|
```go
|
||||||
|
func TaskNewFormHandler(deps TasksDeps) http.HandlerFunc {
|
||||||
|
return func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
tablo, _, ok := loadOwnedTablo(w, r, TablosDeps{Queries: deps.Queries})
|
||||||
|
if !ok {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
w.Header().Set("Content-Type", "text/html; charset=utf-8")
|
||||||
|
_ = templates.TaskCreateFormFragment(...).Render(r.Context(), w)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**HX-Request detection pattern** (lines 150–166 of handlers_tasks.go):
|
||||||
|
```go
|
||||||
|
if r.Header.Get("HX-Request") == "true" {
|
||||||
|
w.Header().Set("Content-Type", "text/html; charset=utf-8")
|
||||||
|
w.Header().Set("HX-Retarget", "#add-task-slot-"+statusStr)
|
||||||
|
w.Header().Set("HX-Reswap", "innerHTML")
|
||||||
|
w.WriteHeader(http.StatusUnprocessableEntity)
|
||||||
|
_ = templates.TaskCreateFormFragment(...).Render(ctx, w)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
http.Redirect(w, r, "/tablos/"+tablo.ID.String(), http.StatusSeeOther)
|
||||||
|
```
|
||||||
|
|
||||||
|
**Delete handler pattern** (lines 285–309 of handlers_tasks.go):
|
||||||
|
```go
|
||||||
|
func TaskDeleteHandler(deps TasksDeps) http.HandlerFunc {
|
||||||
|
return func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
tablo, task, _, ok := loadOwnedTabloForTask(w, r, deps)
|
||||||
|
if !ok {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if err := deps.Queries.DeleteTask(r.Context(), sqlc.DeleteTaskParams{
|
||||||
|
ID: task.ID,
|
||||||
|
TabloID: tablo.ID,
|
||||||
|
}); err != nil {
|
||||||
|
slog.Default().Error("tasks delete: DeleteTask failed", "id", task.ID, "err", err)
|
||||||
|
http.Error(w, "internal server error", http.StatusInternalServerError)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if r.Header.Get("HX-Request") == "true" {
|
||||||
|
w.Header().Set("Content-Type", "text/html; charset=utf-8")
|
||||||
|
_ = templates.TaskCardGone(task.ID).Render(r.Context(), w)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
http.Redirect(w, r, "/tablos/"+tablo.ID.String(), http.StatusSeeOther)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**FileDeleteHandler deviation:** Delete S3 first (log error, do not abort), always delete DB row. The file-specific ordering is: `deps.Files.Delete(ctx, file.S3Key)` → log if error → `deps.Queries.DeleteTabloFile(...)` → 500 if DB fails. See RESEARCH.md Pattern 6.
|
||||||
|
|
||||||
|
**Upload handler additions** not present in tasks.go:
|
||||||
|
- `r.Body = http.MaxBytesReader(w, r.Body, maxBytes)` — MUST be first line (before ParseMultipartForm)
|
||||||
|
- `r.ParseMultipartForm(2 << 20)` + `errors.As(err, &(*http.MaxBytesError)(nil))` for size error detection
|
||||||
|
- `r.FormFile("file")` + `io.ReadFull` sniff + `io.MultiReader` reconstruct
|
||||||
|
- `uuid.New()` for S3 key: `"files/" + tablo.ID.String() + "/" + fileUUID.String()`
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### `backend/internal/web/router.go` (config, request-response — modify)
|
||||||
|
|
||||||
|
**Analog:** `backend/internal/web/router.go` (self)
|
||||||
|
|
||||||
|
**Current NewRouter signature** (line 47):
|
||||||
|
```go
|
||||||
|
func NewRouter(pinger Pinger, staticDir string, deps AuthDeps, tabloDeps TablosDeps, taskDeps TasksDeps, csrfKey []byte, env string, trustedOrigins ...string) http.Handler {
|
||||||
|
```
|
||||||
|
|
||||||
|
**Updated signature** (add `fileDeps FilesDeps` before `csrfKey`):
|
||||||
|
```go
|
||||||
|
func NewRouter(pinger Pinger, staticDir string, deps AuthDeps, tabloDeps TablosDeps, taskDeps TasksDeps, fileDeps FilesDeps, csrfKey []byte, env string, trustedOrigins ...string) http.Handler {
|
||||||
|
```
|
||||||
|
|
||||||
|
**Route registration pattern** (lines 93–106 of router.go — task routes as model):
|
||||||
|
```go
|
||||||
|
// Task routes — static segments BEFORE parametric (Pitfall 1).
|
||||||
|
r.Get("/tablos/{id}/tasks/new", TaskNewFormHandler(taskDeps))
|
||||||
|
r.Get("/tablos/{id}/tasks/cancel-new", TaskCancelNewHandler(taskDeps))
|
||||||
|
r.Post("/tablos/{id}/tasks", TaskCreateHandler(taskDeps))
|
||||||
|
r.Post("/tablos/{id}/tasks/reorder", TaskReorderHandler(taskDeps))
|
||||||
|
// Parametric task routes — must come after static task segments.
|
||||||
|
r.Get("/tablos/{id}/tasks/{task_id}/show", TaskShowHandler(taskDeps))
|
||||||
|
r.Get("/tablos/{id}/tasks/{task_id}/edit", TaskEditHandler(taskDeps))
|
||||||
|
r.Post("/tablos/{id}/tasks/{task_id}", TaskUpdateHandler(taskDeps))
|
||||||
|
r.Get("/tablos/{id}/tasks/{task_id}/delete-confirm", TaskDeleteConfirmHandler(taskDeps))
|
||||||
|
r.Post("/tablos/{id}/tasks/{task_id}/delete", TaskDeleteHandler(taskDeps))
|
||||||
|
```
|
||||||
|
|
||||||
|
**New routes to add** (insert after existing task routes, following same static-before-parametric rule):
|
||||||
|
```go
|
||||||
|
// Tab entry points — GET /tablos/{id}/tasks (Tasks tab) must come BEFORE
|
||||||
|
// /tablos/{id}/tasks/new and other static task sub-routes (chi Pitfall 1).
|
||||||
|
r.Get("/tablos/{id}/tasks", TabloTasksTabHandler(taskDeps)) // NEW — Tasks tab entry
|
||||||
|
|
||||||
|
// File routes — static segments BEFORE parametric (Pitfall 6 in RESEARCH).
|
||||||
|
r.Get("/tablos/{id}/files", TabloFilesTabHandler(fileDeps))
|
||||||
|
r.Post("/tablos/{id}/files", FileUploadHandler(fileDeps))
|
||||||
|
// Parametric file routes — AFTER static file segment.
|
||||||
|
r.Get("/tablos/{id}/files/{file_id}/download", FileDownloadHandler(fileDeps))
|
||||||
|
r.Get("/tablos/{id}/files/{file_id}/delete-confirm", FileDeleteConfirmHandler(fileDeps))
|
||||||
|
r.Post("/tablos/{id}/files/{file_id}/delete", FileDeleteHandler(fileDeps))
|
||||||
|
```
|
||||||
|
|
||||||
|
**IMPORTANT:** `r.Get("/tablos/{id}/tasks", ...)` must be registered BEFORE `r.Get("/tablos/{id}/tasks/new", ...)` — insert it at line 96 (before the existing task block).
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### `backend/templates/tablos.templ` (component, request-response — modify)
|
||||||
|
|
||||||
|
**Analog:** `backend/templates/tablos.templ` (self)
|
||||||
|
|
||||||
|
**Current TabloDetailPage signature** (line 173):
|
||||||
|
```go
|
||||||
|
templ TabloDetailPage(user *auth.User, csrfToken string, tablo sqlc.Tablo, tasks []sqlc.Task) {
|
||||||
|
```
|
||||||
|
|
||||||
|
**Updated signature** (add files + activeTab trailing params — Pitfall 4 in RESEARCH):
|
||||||
|
```go
|
||||||
|
templ TabloDetailPage(user *auth.User, csrfToken string, tablo sqlc.Tablo, tasks []sqlc.Task, files []sqlc.TabloFile, activeTab string) {
|
||||||
|
```
|
||||||
|
|
||||||
|
**All call sites that must be updated:**
|
||||||
|
- `handlers_tablos.go` line 205: `templates.TabloDetailPage(user, csrf.Token(r), tablo, tasks)` → `templates.TabloDetailPage(user, csrf.Token(r), tablo, tasks, nil, "overview")`
|
||||||
|
- `handlers_tablos.go` line 311: same update
|
||||||
|
- Any test files calling `TabloDetailPage`
|
||||||
|
|
||||||
|
**Current detail page body** (lines 174–191) for reference — the tab bar wraps the existing zones:
|
||||||
|
```go
|
||||||
|
templ TabloDetailPage(user *auth.User, csrfToken string, tablo sqlc.Tablo, tasks []sqlc.Task) {
|
||||||
|
@Layout("Tablos — Xtablo", user, csrfToken) {
|
||||||
|
<div class="mb-4">
|
||||||
|
<a href="/" class="text-sm text-slate-600 hover:underline">← Back to tablos</a>
|
||||||
|
</div>
|
||||||
|
<div class="tablo-title-zone">
|
||||||
|
@TabloTitleDisplay(tablo, csrfToken)
|
||||||
|
</div>
|
||||||
|
<div class="tablo-desc-zone">
|
||||||
|
@TabloDescDisplay(tablo, csrfToken)
|
||||||
|
</div>
|
||||||
|
<div class="tablo-delete-zone">
|
||||||
|
@TabloDeleteButtonFragment(tablo, csrfToken)
|
||||||
|
</div>
|
||||||
|
<div class="mt-8">
|
||||||
|
@KanbanBoard(tablo.ID, csrfToken, tasks)
|
||||||
|
</div>
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Tab bar pattern** — insert between delete-zone and content area. Tab links use `hx-get` + `hx-target="#tab-content"` + `hx-push-url`:
|
||||||
|
```html
|
||||||
|
<nav class="mt-8 flex border-b border-slate-200">
|
||||||
|
<a
|
||||||
|
href={ templ.SafeURL("/tablos/" + tablo.ID.String()) }
|
||||||
|
hx-get={ "/tablos/" + tablo.ID.String() }
|
||||||
|
hx-target="#tab-content"
|
||||||
|
hx-swap="innerHTML"
|
||||||
|
hx-push-url={ "/tablos/" + tablo.ID.String() }
|
||||||
|
class="tab-link ..."
|
||||||
|
>Overview</a>
|
||||||
|
<!-- Tasks and Files tabs follow same shape -->
|
||||||
|
</nav>
|
||||||
|
<div id="tab-content" class="mt-6">
|
||||||
|
<!-- dispatch on activeTab -->
|
||||||
|
</div>
|
||||||
|
```
|
||||||
|
|
||||||
|
**Fragment-only components needed:** `TabloOverviewTabFragment`, `TabloTasksTabFragment`, `TabloFilesTabFragment` — each is what the `#tab-content` div contains when that tab is active. These are rendered either as the full-page content area (initial load) or as standalone fragments (HTMX request).
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### `backend/templates/files.templ` (component, request-response — new)
|
||||||
|
|
||||||
|
**Analog:** `backend/templates/tasks.templ`
|
||||||
|
|
||||||
|
**Package/import pattern** (lines 1–8 of tasks.templ):
|
||||||
|
```go
|
||||||
|
package templates
|
||||||
|
|
||||||
|
import (
|
||||||
|
"backend/internal/db/sqlc"
|
||||||
|
"backend/internal/web/ui"
|
||||||
|
"github.com/google/uuid"
|
||||||
|
)
|
||||||
|
```
|
||||||
|
|
||||||
|
**Delete confirmation pattern** (lines 285–326 of tasks.templ — copy structure for file delete confirm):
|
||||||
|
```go
|
||||||
|
templ TaskDeleteConfirmFragment(tabloID uuid.UUID, task sqlc.Task, csrfToken string) {
|
||||||
|
<div class="task-card-zone" id={ "task-" + task.ID.String() }>
|
||||||
|
<div class="bg-white rounded border border-slate-200 p-3 shadow-sm space-y-2">
|
||||||
|
<p class="text-sm font-semibold text-slate-800">Delete task?</p>
|
||||||
|
<p class="text-xs text-slate-600">This cannot be undone.</p>
|
||||||
|
<div class="flex items-center gap-2">
|
||||||
|
<form
|
||||||
|
method="POST"
|
||||||
|
action={ templ.SafeURL("/tablos/" + tabloID.String() + "/tasks/" + task.ID.String() + "/delete") }
|
||||||
|
hx-post={ ... }
|
||||||
|
hx-target="closest .task-card-zone"
|
||||||
|
hx-swap="outerHTML"
|
||||||
|
>
|
||||||
|
@ui.CSRFField(csrfToken)
|
||||||
|
@ui.Button(ui.ButtonProps{
|
||||||
|
Label: "Yes, delete",
|
||||||
|
Variant: ui.ButtonVariantDanger,
|
||||||
|
Tone: ui.ButtonToneSolid,
|
||||||
|
...
|
||||||
|
})
|
||||||
|
</form>
|
||||||
|
@ui.Button(ui.ButtonProps{
|
||||||
|
Label: "Keep task",
|
||||||
|
Variant: ui.ButtonVariantNeutral,
|
||||||
|
Tone: ui.ButtonToneSoft,
|
||||||
|
...
|
||||||
|
})
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**ui.Card pattern** for file list rows (from tablos.templ lines 70–93):
|
||||||
|
```go
|
||||||
|
@ui.Card(templ.Attributes{"id": "tablo-" + tablo.ID.String()}) {
|
||||||
|
<div class="flex items-start justify-between">
|
||||||
|
...
|
||||||
|
</div>
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**OOB removal pattern** (from TaskCardGone, lines 343–345 of tasks.templ):
|
||||||
|
```go
|
||||||
|
templ TaskCardGone(taskID uuid.UUID) {
|
||||||
|
<div id={ "task-" + taskID.String() } class="task-card-zone"></div>
|
||||||
|
}
|
||||||
|
```
|
||||||
|
File analog: `FileRowGone(fileID uuid.UUID)` — same empty-div-with-id pattern.
|
||||||
|
|
||||||
|
**Upload form** uses `method="POST"` + `enctype="multipart/form-data"` + `hx-post` + `hx-encoding="multipart/form-data"` (HTMX multipart requires this attribute). Input: `<input type="file" name="file" ...>`. Add `@ui.CSRFField(csrfToken)`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### `backend/cmd/web/main.go` (config — modify)
|
||||||
|
|
||||||
|
**Analog:** `backend/cmd/web/main.go` (self)
|
||||||
|
|
||||||
|
**Current deps wiring** (lines 78–82):
|
||||||
|
```go
|
||||||
|
deps := web.AuthDeps{Queries: q, Store: store, Secure: secure, Limiter: rl}
|
||||||
|
tabloDeps := web.TablosDeps{Queries: q}
|
||||||
|
taskDeps := web.TasksDeps{Queries: q}
|
||||||
|
|
||||||
|
router := web.NewRouter(pool, "./static", deps, tabloDeps, taskDeps, csrfKey, env)
|
||||||
|
```
|
||||||
|
|
||||||
|
**Pattern for reading env vars** (lines 27–35 — copy pattern for S3 env vars):
|
||||||
|
```go
|
||||||
|
env := os.Getenv("ENV")
|
||||||
|
if env == "" {
|
||||||
|
env = "development"
|
||||||
|
}
|
||||||
|
port := os.Getenv("PORT")
|
||||||
|
if port == "" {
|
||||||
|
port = "8080"
|
||||||
|
}
|
||||||
|
dsn := os.Getenv("DATABASE_URL")
|
||||||
|
```
|
||||||
|
|
||||||
|
**Fast-fail pattern for required vars** (lines 41–44):
|
||||||
|
```go
|
||||||
|
if dsn == "" {
|
||||||
|
slog.Error("DATABASE_URL is required but unset")
|
||||||
|
os.Exit(1)
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**S3 wiring to add** (insert before `router := ...`):
|
||||||
|
```go
|
||||||
|
// S3 / files store
|
||||||
|
s3Endpoint := os.Getenv("S3_ENDPOINT")
|
||||||
|
s3Bucket := os.Getenv("S3_BUCKET")
|
||||||
|
// ... read remaining vars ...
|
||||||
|
maxUploadMB := 25
|
||||||
|
if v := os.Getenv("MAX_UPLOAD_SIZE_MB"); v != "" {
|
||||||
|
// strconv.Atoi + fallback
|
||||||
|
}
|
||||||
|
filesStore, err := files.NewStore(ctx, s3Endpoint, s3Bucket, ...)
|
||||||
|
if err != nil {
|
||||||
|
slog.Error("s3 client init failed", "err", err)
|
||||||
|
os.Exit(1)
|
||||||
|
}
|
||||||
|
fileDeps := web.FilesDeps{Queries: q, Files: filesStore, MaxUploadMB: maxUploadMB}
|
||||||
|
|
||||||
|
router := web.NewRouter(pool, "./static", deps, tabloDeps, taskDeps, fileDeps, csrfKey, env)
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### `compose.yaml` (config — modify)
|
||||||
|
|
||||||
|
**Analog:** `backend/compose.yaml` (self)
|
||||||
|
|
||||||
|
**Existing postgres service** (lines 1–20 — copy structure exactly):
|
||||||
|
```yaml
|
||||||
|
services:
|
||||||
|
postgres:
|
||||||
|
image: postgres:16-alpine
|
||||||
|
container_name: xtablo-backend-postgres
|
||||||
|
restart: unless-stopped
|
||||||
|
environment:
|
||||||
|
POSTGRES_DB: xtablo
|
||||||
|
POSTGRES_USER: xtablo
|
||||||
|
POSTGRES_PASSWORD: xtablo
|
||||||
|
ports:
|
||||||
|
- "5432:5432"
|
||||||
|
volumes:
|
||||||
|
- postgres_data:/var/lib/postgresql/data
|
||||||
|
healthcheck:
|
||||||
|
test: ["CMD-SHELL", "pg_isready -U xtablo -d xtablo"]
|
||||||
|
interval: 5s
|
||||||
|
timeout: 5s
|
||||||
|
retries: 10
|
||||||
|
|
||||||
|
volumes:
|
||||||
|
postgres_data:
|
||||||
|
```
|
||||||
|
|
||||||
|
**MinIO services to add** (per RESEARCH.md Pattern 7 — verified against MinIO Docker Hub):
|
||||||
|
```yaml
|
||||||
|
minio:
|
||||||
|
image: minio/minio:latest
|
||||||
|
container_name: xtablo-backend-minio
|
||||||
|
restart: unless-stopped
|
||||||
|
environment:
|
||||||
|
MINIO_ROOT_USER: minioadmin
|
||||||
|
MINIO_ROOT_PASSWORD: minioadmin
|
||||||
|
ports:
|
||||||
|
- "9000:9000" # S3 API
|
||||||
|
- "9001:9001" # Console UI
|
||||||
|
command: server /data --console-address ":9001"
|
||||||
|
volumes:
|
||||||
|
- minio_data:/data
|
||||||
|
healthcheck:
|
||||||
|
test: ["CMD", "mc", "ready", "local"]
|
||||||
|
interval: 5s
|
||||||
|
timeout: 5s
|
||||||
|
retries: 10
|
||||||
|
|
||||||
|
minio-init:
|
||||||
|
image: minio/mc:latest
|
||||||
|
depends_on:
|
||||||
|
minio:
|
||||||
|
condition: service_healthy
|
||||||
|
entrypoint: >
|
||||||
|
/bin/sh -c "
|
||||||
|
mc alias set local http://minio:9000 minioadmin minioadmin;
|
||||||
|
mc mb --ignore-existing local/xtablo-dev;
|
||||||
|
echo 'bucket ready';
|
||||||
|
"
|
||||||
|
restart: "no" # init container — must NOT be unless-stopped (Pitfall 7)
|
||||||
|
```
|
||||||
|
|
||||||
|
**Add `minio_data:` under `volumes:` block alongside `postgres_data:`.**
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Shared Patterns
|
||||||
|
|
||||||
|
### Auth / Ownership (loadOwnedTablo)
|
||||||
|
**Source:** `backend/internal/web/handlers_tablos.go` lines 150–182
|
||||||
|
**Apply to:** All file handlers (TabloFilesTabHandler, FileUploadHandler, FileDownloadHandler, FileDeleteHandler, FileDeleteConfirmHandler)
|
||||||
|
|
||||||
|
```go
|
||||||
|
func loadOwnedTablo(w http.ResponseWriter, r *http.Request, deps TablosDeps) (sqlc.Tablo, *auth.User, bool) {
|
||||||
|
_, user, _ := auth.Authed(r.Context())
|
||||||
|
tabloID, err := uuid.Parse(chi.URLParam(r, "id"))
|
||||||
|
if err != nil {
|
||||||
|
http.NotFound(w, r)
|
||||||
|
return sqlc.Tablo{}, nil, false
|
||||||
|
}
|
||||||
|
tablo, err := deps.Queries.GetTabloByID(r.Context(), sqlc.GetTabloByIDParams{ID: tabloID, UserID: user.ID})
|
||||||
|
if err != nil {
|
||||||
|
if errors.Is(err, pgx.ErrNoRows) {
|
||||||
|
http.NotFound(w, r)
|
||||||
|
return sqlc.Tablo{}, nil, false
|
||||||
|
}
|
||||||
|
slog.Default().Error("tablos: GetTabloByID failed", "id", tabloID, "err", err)
|
||||||
|
http.Error(w, "internal server error", http.StatusInternalServerError)
|
||||||
|
return sqlc.Tablo{}, nil, false
|
||||||
|
}
|
||||||
|
return tablo, user, true
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
### Error Logging
|
||||||
|
**Source:** `backend/internal/web/handlers_tasks.go` (throughout)
|
||||||
|
**Apply to:** All handler error paths
|
||||||
|
|
||||||
|
```go
|
||||||
|
slog.Default().Error("files upload: PutObject failed", "tablo_id", tablo.ID, "err", err)
|
||||||
|
http.Error(w, "internal server error", http.StatusInternalServerError)
|
||||||
|
```
|
||||||
|
Convention: log key is `"<domain> <action>: <operation> failed"`. Always include `"tablo_id"` and `"err"` keys.
|
||||||
|
|
||||||
|
### HTMX Fragment vs Full-Page
|
||||||
|
**Source:** `backend/internal/web/handlers_tasks.go` (throughout), `backend/internal/web/handlers_tablos.go`
|
||||||
|
**Apply to:** TabloFilesTabHandler, TabloTasksTabHandler (tab entry points)
|
||||||
|
|
||||||
|
```go
|
||||||
|
w.Header().Set("Content-Type", "text/html; charset=utf-8")
|
||||||
|
if r.Header.Get("HX-Request") == "true" {
|
||||||
|
_ = templates.FilesTabFragment(tablo, files, csrf.Token(r)).Render(r.Context(), w)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
// Full page for direct navigation / browser refresh
|
||||||
|
_ = templates.TabloDetailPage(user, csrf.Token(r), tablo, tasks, files, "files").Render(r.Context(), w)
|
||||||
|
```
|
||||||
|
|
||||||
|
### CSRF Field in All Forms
|
||||||
|
**Source:** `backend/templates/tablos.templ` line 108, `backend/templates/tasks.templ` lines 35, 183, 244
|
||||||
|
**Apply to:** All POST forms in files.templ (upload form, delete confirm form)
|
||||||
|
|
||||||
|
```go
|
||||||
|
@ui.CSRFField(csrfToken)
|
||||||
|
```
|
||||||
|
|
||||||
|
### Post/Redirect/Get (non-HTMX fallback)
|
||||||
|
**Source:** `backend/internal/web/handlers_tasks.go` line 248, 307
|
||||||
|
**Apply to:** FileUploadHandler, FileDeleteHandler non-HTMX paths
|
||||||
|
|
||||||
|
```go
|
||||||
|
http.Redirect(w, r, "/tablos/"+tablo.ID.String()+"/files", http.StatusSeeOther)
|
||||||
|
// Always 303 SeeOther on POST success — NOT 302 (Pitfall 9)
|
||||||
|
```
|
||||||
|
|
||||||
|
### ui.Button component usage
|
||||||
|
**Source:** `backend/templates/tablos.templ` lines 339–353, `backend/templates/tasks.templ` lines 258–278
|
||||||
|
**Apply to:** Upload form submit button, delete confirm buttons in files.templ
|
||||||
|
|
||||||
|
```go
|
||||||
|
@ui.Button(ui.ButtonProps{
|
||||||
|
Label: "Upload",
|
||||||
|
Variant: ui.ButtonVariantDefault,
|
||||||
|
Tone: ui.ButtonToneSolid,
|
||||||
|
Size: ui.SizeMD,
|
||||||
|
Type: "submit",
|
||||||
|
})
|
||||||
|
@ui.Button(ui.ButtonProps{
|
||||||
|
Label: "Yes, delete",
|
||||||
|
Variant: ui.ButtonVariantDanger,
|
||||||
|
Tone: ui.ButtonToneSolid,
|
||||||
|
Size: ui.SizeMD,
|
||||||
|
Type: "submit",
|
||||||
|
})
|
||||||
|
```
|
||||||
|
|
||||||
|
### templ.SafeURL for action attributes
|
||||||
|
**Source:** `backend/templates/tablos.templ` lines 217, 367; `backend/templates/tasks.templ` line 177
|
||||||
|
**Apply to:** All `action=` attributes in files.templ forms
|
||||||
|
|
||||||
|
```go
|
||||||
|
action={ templ.SafeURL("/tablos/" + tablo.ID.String() + "/files") }
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## No Analog Found
|
||||||
|
|
||||||
|
No files in this phase lack a codebase analog. `backend/internal/files/store.go` has no body analog, but RESEARCH.md Patterns 1–3 provide verified, concrete implementation patterns with line-level specificity.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Metadata
|
||||||
|
|
||||||
|
**Analog search scope:** `backend/` directory — migrations, queries, handlers, templates, cmd, compose.yaml
|
||||||
|
**Files scanned:** 12 source files read directly
|
||||||
|
**Pattern extraction date:** 2026-05-15
|
||||||
|
|
@ -679,17 +679,19 @@ MAX_UPLOAD_SIZE_MB=25
|
||||||
| A3 | `multipart.FileHeader.Size` is unreliable (some clients send 0) | Pitfall 8, Code Examples | If reliable, a counting wrapper is unnecessary; but safest to always count bytes written |
|
| A3 | `multipart.FileHeader.Size` is unreliable (some clients send 0) | Pitfall 8, Code Examples | If reliable, a counting wrapper is unnecessary; but safest to always count bytes written |
|
||||||
| A4 | MinIO `latest` image includes `mc` binary (for healthcheck `mc ready local`) | Pattern 7 compose.yaml | `mc` may not be bundled in `minio/minio` image; separate `minio/mc` init container is safer (used in Pattern 7) |
|
| A4 | MinIO `latest` image includes `mc` binary (for healthcheck `mc ready local`) | Pattern 7 compose.yaml | `mc` may not be bundled in `minio/minio` image; separate `minio/mc` init container is safer (used in Pattern 7) |
|
||||||
|
|
||||||
## Open Questions
|
## Open Questions (RESOLVED)
|
||||||
|
|
||||||
1. **Tab route for `/tablos/{id}/tasks` conflicts with existing `/tablos/{id}/tasks/*` child routes**
|
1. **Tab route for `/tablos/{id}/tasks` conflicts with existing `/tablos/{id}/tasks/*` child routes**
|
||||||
- What we know: `GET /tablos/{id}/tasks` is NOT currently registered. All task routes are `GET/POST /tablos/{id}/tasks/...` (with a trailing segment).
|
- What we know: `GET /tablos/{id}/tasks` is NOT currently registered. All task routes are `GET/POST /tablos/{id}/tasks/...` (with a trailing segment).
|
||||||
- What's unclear: chi v5's behavior when registering `GET /tablos/{id}/tasks` alongside `GET /tablos/{id}/tasks/new` and `POST /tablos/{id}/tasks/reorder`. Static + trailing segments should be fine, but the exact chi precedence should be verified.
|
- What's unclear: chi v5's behavior when registering `GET /tablos/{id}/tasks` alongside `GET /tablos/{id}/tasks/new` and `POST /tablos/{id}/tasks/reorder`. Static + trailing segments should be fine, but the exact chi precedence should be verified.
|
||||||
- Recommendation: Add `r.Get("/tablos/{id}/tasks", TabloTasksTabHandler(...))` BEFORE the existing task sub-routes. Write a test that `GET /tablos/{id}/tasks` returns 200 with the kanban board, and `GET /tablos/{id}/tasks/new` still returns the new-task form.
|
- Recommendation: Add `r.Get("/tablos/{id}/tasks", TabloTasksTabHandler(...))` BEFORE the existing task sub-routes. Write a test that `GET /tablos/{id}/tasks` returns 200 with the kanban board, and `GET /tablos/{id}/tasks/new` still returns the new-task form.
|
||||||
|
- **RESOLVED:** 05-02 Task 2 step 5 registers `GET /tablos/{id}/tasks` (via `TabloTasksTabHandler`) before existing task sub-routes in router.go, following chi's static-before-parametric discipline. Route ordering is enforced in the plan action.
|
||||||
|
|
||||||
2. **Byte-counting for `size_bytes` column**
|
2. **Byte-counting for `size_bytes` column**
|
||||||
- What we know: `s3.PutObject` does not return the number of bytes written. `multipart.FileHeader.Size` may be 0.
|
- What we know: `s3.PutObject` does not return the number of bytes written. `multipart.FileHeader.Size` may be 0.
|
||||||
- What's unclear: The cleanest counting approach (wrap body in a custom `io.Reader` that increments a counter, vs. use `header.Size` as-is and accept that it may be browser-dependent).
|
- What's unclear: The cleanest counting approach (wrap body in a custom `io.Reader` that increments a counter, vs. use `header.Size` as-is and accept that it may be browser-dependent).
|
||||||
- Recommendation: Wrap the body with a simple `byteCountReader` before passing to `PutObject`. After upload, use the counter value for `InsertTabloFileParams.SizeBytes`.
|
- Recommendation: Wrap the body with a simple `byteCountReader` before passing to `PutObject`. After upload, use the counter value for `InsertTabloFileParams.SizeBytes`.
|
||||||
|
- **RESOLVED:** 05-01 Task 1 step 5 implements `byteCountReader` in `store.go`, wrapping the multipart body before the `PutObject` call. Counter value is passed as `SizeBytes` in `InsertTabloFileParams`.
|
||||||
|
|
||||||
## Environment Availability
|
## Environment Availability
|
||||||
|
|
||||||
|
|
|
||||||
84
.planning/phases/05-files/05-VALIDATION.md
Normal file
84
.planning/phases/05-files/05-VALIDATION.md
Normal file
|
|
@ -0,0 +1,84 @@
|
||||||
|
---
|
||||||
|
phase: 5
|
||||||
|
slug: files
|
||||||
|
status: draft
|
||||||
|
nyquist_compliant: false
|
||||||
|
wave_0_complete: false
|
||||||
|
created: 2026-05-15
|
||||||
|
---
|
||||||
|
|
||||||
|
# Phase 5 — Validation Strategy
|
||||||
|
|
||||||
|
> Per-phase validation contract for feedback sampling during execution.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Test Infrastructure
|
||||||
|
|
||||||
|
| Property | Value |
|
||||||
|
|----------|-------|
|
||||||
|
| **Framework** | go test |
|
||||||
|
| **Config file** | none — built into Go toolchain |
|
||||||
|
| **Quick run command** | `cd backend && go test ./internal/... -run TestFile -timeout 30s` |
|
||||||
|
| **Full suite command** | `cd backend && go test ./... -timeout 60s` |
|
||||||
|
| **Estimated runtime** | ~15 seconds |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Sampling Rate
|
||||||
|
|
||||||
|
- **After every task commit:** Run `cd backend && go test ./internal/... -run TestFile -timeout 30s`
|
||||||
|
- **After every plan wave:** Run `cd backend && go test ./... -timeout 60s`
|
||||||
|
- **Before `/gsd-verify-work`:** Full suite must be green
|
||||||
|
- **Max feedback latency:** 60 seconds
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Per-Task Verification Map
|
||||||
|
|
||||||
|
| Task ID | Plan | Wave | Requirement | Threat Ref | Secure Behavior | Test Type | Automated Command | File Exists | Status |
|
||||||
|
|---------|------|------|-------------|------------|-----------------|-----------|-------------------|-------------|--------|
|
||||||
|
| 05-migration | — | 1 | FILE-01 | — | tablo_files schema correct | unit | `cd backend && go test ./internal/db/...` | ❌ W0 | ⬜ pending |
|
||||||
|
| 05-s3-client | — | 1 | FILE-02 | — | S3 client connects to MinIO | unit | `cd backend && go test ./internal/files/...` | ❌ W0 | ⬜ pending |
|
||||||
|
| 05-upload | — | 1 | FILE-02 | — | MaxBytesReader enforced, bytes stored | unit | `cd backend && go test ./internal/files/... -run TestUpload` | ❌ W0 | ⬜ pending |
|
||||||
|
| 05-list | — | 1 | FILE-03 | — | Files listed newest-first | unit | `cd backend && go test ./internal/web/... -run TestFilesList` | ❌ W0 | ⬜ pending |
|
||||||
|
| 05-download | — | 1 | FILE-04 | — | Signed URL generated, 302 redirect | unit | `cd backend && go test ./internal/web/... -run TestFilesDownload` | ❌ W0 | ⬜ pending |
|
||||||
|
| 05-delete | — | 1 | FILE-05 | — | DB row and S3 object both removed | unit | `cd backend && go test ./internal/web/... -run TestFilesDelete` | ❌ W0 | ⬜ pending |
|
||||||
|
| 05-authz | — | 2 | FILE-06 | — | Non-owner gets 403 | unit | `cd backend && go test ./internal/web/... -run TestFilesOwnership` | ❌ W0 | ⬜ pending |
|
||||||
|
| 05-tabs | — | 2 | FILE-01 | — | Tab routes render correct fragment | unit | `cd backend && go test ./internal/web/... -run TestTabloDetailTabs` | ❌ W0 | ⬜ pending |
|
||||||
|
|
||||||
|
*Status: ⬜ pending · ✅ green · ❌ red · ⚠️ flaky*
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Wave 0 Requirements
|
||||||
|
|
||||||
|
- [ ] `backend/internal/files/store_test.go` — stubs for FILE-02, FILE-04, FILE-05
|
||||||
|
- [ ] `backend/internal/web/handlers_files_test.go` — stubs for FILE-01, FILE-03, FILE-06
|
||||||
|
- [ ] MinIO running in compose.yaml — integration tests need the local S3 endpoint
|
||||||
|
|
||||||
|
*If none: "Existing infrastructure covers all phase requirements."*
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Manual-Only Verifications
|
||||||
|
|
||||||
|
| Behavior | Requirement | Why Manual | Test Instructions |
|
||||||
|
|----------|-------------|------------|-------------------|
|
||||||
|
| File upload form renders correctly in browser | FILE-01 | Visual layout cannot be asserted in go test | Open `/tablos/{id}/files`, attach a file, verify form submits and file appears in list |
|
||||||
|
| Tab navigation updates URL via hx-push-url | FILE-01 | Browser URL state not testable in unit tests | Click Tasks tab, verify URL changes to `/tablos/{id}/tasks`; hit Back, verify Overview tab reactivates |
|
||||||
|
| 25MB upload size enforcement shows friendly error | FILE-02 | Browser file size display is visual | Attempt to upload a >25MB file; verify error message appears above the form |
|
||||||
|
| Download opens file correctly via 302 redirect | FILE-04 | Actual browser navigation/download | Click download link; verify browser downloads the file within signed URL TTL |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Validation Sign-Off
|
||||||
|
|
||||||
|
- [ ] All tasks have `<automated>` verify or Wave 0 dependencies
|
||||||
|
- [ ] Sampling continuity: no 3 consecutive tasks without automated verify
|
||||||
|
- [ ] Wave 0 covers all MISSING references
|
||||||
|
- [ ] No watch-mode flags
|
||||||
|
- [ ] Feedback latency < 60s
|
||||||
|
- [ ] `nyquist_compliant: true` set in frontmatter
|
||||||
|
|
||||||
|
**Approval:** pending
|
||||||
Loading…
Reference in a new issue