refactor(uploads): drop legacy multipart code paths
Backend CI / Test (push) Has been cancelled
Backend CI / Contract Tests (push) Has been cancelled
Backend CI / Build (push) Has been cancelled
Backend CI / Lint (push) Has been cancelled
Backend CI / Secret Scanning (push) Has been cancelled

The presigned-URL upload flow (POST /api/uploads/presign + direct B2 POST
+ upload_ids[] in entity creation) is now the only image upload path. The
legacy multipart routes and DTO fields used by older clients are removed:

Removed:
  - POST /api/uploads/image/        (legacy multipart upload → URL)
  - POST /api/uploads/document/     (legacy multipart upload → URL)
  - POST /api/uploads/completion/   (legacy multipart upload → URL)
  - Multipart branch in POST /api/task-completions/ (now JSON-only)
  - CreateTaskCompletionRequest.ImageURLs DTO field
  - UpdateTaskCompletionRequest.ImageURLs DTO field
  - CreateDocumentRequest.ImageURLs DTO field
  - Service-layer ImageURLs loops in task_service.CreateCompletion,
    task_service.UpdateCompletion, document_service.CreateDocument
  - Tests exercising the removed paths
  - Now-unused imports (strings/time/decimal) in task_handler.go

Kept:
  - DELETE /api/uploads/  (orphan-cleanup endpoint, still useful)
  - POST /api/uploads/presign/  (the new path)
  - POST /api/documents/:id/images/  (uses storage_service.Upload directly,
    same multipart pattern but separate code path; deferred for now)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Trey t
2026-05-01 15:19:21 -07:00
parent 29c9014a33
commit b7f83293b8
9 changed files with 43 additions and 261 deletions
+4 -5
View File
@@ -25,12 +25,11 @@ type CreateDocumentRequest struct {
SerialNumber string `json:"serial_number" validate:"max=100"` SerialNumber string `json:"serial_number" validate:"max=100"`
ModelNumber string `json:"model_number" validate:"max=100"` ModelNumber string `json:"model_number" validate:"max=100"`
TaskID *uint `json:"task_id"` TaskID *uint `json:"task_id"`
ImageURLs []string `json:"image_urls" validate:"omitempty,max=20,dive,max=500"` // Legacy multipart upload path
// UploadIDs claims pending_uploads rows produced by the presigned-URL // UploadIDs claims pending_uploads rows produced by the presigned-URL
// upload flow and turns them into document_image rows. May be combined // upload flow and turns them into document_image rows. UploadIDs of
// with ImageURLs during the rollout window. UploadIDs of category // category "document_file" attach to the document's main FileURL +
// "document_file" attach to the document's main FileURL/FileName fields // FileName fields instead — the service infers placement from the
// instead — the service infers placement from the row's category. // row's category.
UploadIDs []uint `json:"upload_ids" validate:"omitempty,max=20"` UploadIDs []uint `json:"upload_ids" validate:"omitempty,max=20"`
} }
+6 -15
View File
@@ -100,7 +100,12 @@ type UpdateTaskRequest struct {
ContractorID *uint `json:"contractor_id"` ContractorID *uint `json:"contractor_id"`
} }
// CreateTaskCompletionRequest represents the request to create a task completion // CreateTaskCompletionRequest represents the request to create a task completion.
//
// Image attachments arrive via the presigned-URL flow: the client uploads
// each image directly to B2 (see /api/uploads/presign) and passes the
// resulting pending_uploads.id values in UploadIDs. The service claims
// those rows and creates the linked task_completion_image rows.
type CreateTaskCompletionRequest struct { type CreateTaskCompletionRequest struct {
TaskID uint `json:"task_id" validate:"required"` TaskID uint `json:"task_id" validate:"required"`
CompletedAt *time.Time `json:"completed_at"` // Defaults to now CompletedAt *time.Time `json:"completed_at"` // Defaults to now
@@ -108,19 +113,6 @@ type CreateTaskCompletionRequest struct {
ActualCost *decimal.Decimal `json:"actual_cost"` ActualCost *decimal.Decimal `json:"actual_cost"`
Rating *int `json:"rating" validate:"omitempty,min=1,max=5"` // 1-5 star rating Rating *int `json:"rating" validate:"omitempty,min=1,max=5"` // 1-5 star rating
// ImageURLs is the legacy multipart-upload path: the handler uploaded the
// images first via the same request and produced URLs. Still supported for
// older client builds.
ImageURLs []string `json:"image_urls" validate:"omitempty,max=20,dive,max=500"`
// UploadIDs is the new direct-to-B2 path: the client uploaded each image
// via a presigned URL and now claims the resulting pending_uploads rows
// by id. The service verifies ownership + size, marks each row claimed,
// and creates task_completion_image rows from them.
//
// If both ImageURLs and UploadIDs are present, both contribute to the
// final set of images so a single completion can mix legacy and new
// uploads (helps during the rollout window).
UploadIDs []uint `json:"upload_ids" validate:"omitempty,max=20"` UploadIDs []uint `json:"upload_ids" validate:"omitempty,max=20"`
} }
@@ -129,7 +121,6 @@ type UpdateTaskCompletionRequest struct {
Notes *string `json:"notes" validate:"omitempty,max=10000"` Notes *string `json:"notes" validate:"omitempty,max=10000"`
ActualCost *decimal.Decimal `json:"actual_cost"` ActualCost *decimal.Decimal `json:"actual_cost"`
Rating *int `json:"rating" validate:"omitempty,min=1,max=5"` Rating *int `json:"rating" validate:"omitempty,min=1,max=5"`
ImageURLs []string `json:"image_urls" validate:"omitempty,max=20,dive,max=500"`
} }
// CompletionImageInput represents an image to add to a completion // CompletionImageInput represents an image to add to a completion
+5 -39
View File
@@ -1781,45 +1781,11 @@ func TestStaticDataHandler_RefreshStaticData(t *testing.T) {
// ============================================================================= // =============================================================================
// Upload Handler - Additional Error Paths // Upload Handler - Additional Error Paths
// ============================================================================= // =============================================================================
//
func TestUploadHandler_UploadImage_NoFile(t *testing.T) { // Multipart upload handlers (UploadImage / UploadDocument / UploadCompletion)
storageSvc := newTestStorageService("/var/uploads") // were removed alongside the legacy /api/uploads/{image,document,completion}
handler := NewUploadHandler(storageSvc, nil) // routes. The presigned-URL flow (POST /api/uploads/presign) is exercised by
e := testutil.SetupTestRouter() // integration tests that hit the full pipeline.
e.POST("/api/uploads/image", handler.UploadImage)
t.Run("no file returns 400", func(t *testing.T) {
w := testutil.MakeRequest(e, "POST", "/api/uploads/image", nil, "")
testutil.AssertStatusCode(t, w, http.StatusBadRequest)
})
}
func TestUploadHandler_UploadDocument_NoFile(t *testing.T) {
storageSvc := newTestStorageService("/var/uploads")
handler := NewUploadHandler(storageSvc, nil)
e := testutil.SetupTestRouter()
e.POST("/api/uploads/document", handler.UploadDocument)
t.Run("no file returns 400", func(t *testing.T) {
w := testutil.MakeRequest(e, "POST", "/api/uploads/document", nil, "")
testutil.AssertStatusCode(t, w, http.StatusBadRequest)
})
}
func TestUploadHandler_UploadCompletion_NoFile(t *testing.T) {
storageSvc := newTestStorageService("/var/uploads")
handler := NewUploadHandler(storageSvc, nil)
e := testutil.SetupTestRouter()
e.POST("/api/uploads/completion", handler.UploadCompletion)
t.Run("no file returns 400", func(t *testing.T) {
w := testutil.MakeRequest(e, "POST", "/api/uploads/completion", nil, "")
testutil.AssertStatusCode(t, w, http.StatusBadRequest)
})
}
func TestUploadHandler_DeleteFile_OwnershipDenied(t *testing.T) { func TestUploadHandler_DeleteFile_OwnershipDenied(t *testing.T) {
storageSvc := newTestStorageService("/var/uploads") storageSvc := newTestStorageService("/var/uploads")
+12 -60
View File
@@ -3,11 +3,8 @@ package handlers
import ( import (
"net/http" "net/http"
"strconv" "strconv"
"strings"
"time"
"github.com/labstack/echo/v4" "github.com/labstack/echo/v4"
"github.com/shopspring/decimal"
"github.com/treytartt/honeydue-api/internal/apperrors" "github.com/treytartt/honeydue-api/internal/apperrors"
"github.com/treytartt/honeydue-api/internal/dto/requests" "github.com/treytartt/honeydue-api/internal/dto/requests"
@@ -393,7 +390,18 @@ func (h *TaskHandler) GetCompletion(c echo.Context) error {
} }
// CreateCompletion handles POST /api/task-completions/ // CreateCompletion handles POST /api/task-completions/
// Supports both JSON and multipart form data (for image uploads) //
// JSON-only. Image attachments arrive via the presigned-URL flow:
//
// 1. Client POSTs /api/uploads/presign for each image and uploads bytes
// directly to B2 using the returned policy.
// 2. Client POSTs the resulting upload_ids[] in this request body.
// 3. The service claims those pending_uploads rows and creates the
// associated TaskCompletionImage rows.
//
// The legacy multipart path (with the API server proxying image bytes)
// was removed alongside the 1 MB BodyLimit middleware that was rejecting
// it anyway. See deploy-k3s/manifests/b2-lifecycle.md.
func (h *TaskHandler) CreateCompletion(c echo.Context) error { func (h *TaskHandler) CreateCompletion(c echo.Context) error {
user, err := middleware.MustGetAuthUser(c) user, err := middleware.MustGetAuthUser(c)
if err != nil { if err != nil {
@@ -402,65 +410,9 @@ func (h *TaskHandler) CreateCompletion(c echo.Context) error {
userNow := middleware.GetUserNow(c) userNow := middleware.GetUserNow(c)
var req requests.CreateTaskCompletionRequest var req requests.CreateTaskCompletionRequest
contentType := c.Request().Header.Get("Content-Type")
// Check if this is a multipart form request (image upload)
if strings.HasPrefix(contentType, "multipart/form-data") {
// Parse multipart form
if err := c.Request().ParseMultipartForm(32 << 20); err != nil { // 32MB max
return apperrors.BadRequest("error.failed_to_parse_form")
}
// Parse task_id (required)
taskIDStr := c.FormValue("task_id")
if taskIDStr == "" {
return apperrors.BadRequest("error.task_id_required")
}
taskID, err := strconv.ParseUint(taskIDStr, 10, 32)
if err != nil {
return apperrors.BadRequest("error.invalid_task_id_value")
}
req.TaskID = uint(taskID)
// Parse notes (optional)
req.Notes = c.FormValue("notes")
// Parse actual_cost (optional)
if costStr := c.FormValue("actual_cost"); costStr != "" {
cost, err := decimal.NewFromString(costStr)
if err == nil {
req.ActualCost = &cost
}
}
// Parse completed_at (optional)
if completedAtStr := c.FormValue("completed_at"); completedAtStr != "" {
if t, err := time.Parse(time.RFC3339, completedAtStr); err == nil {
req.CompletedAt = &t
}
}
// Handle multiple image uploads from various field names
if h.storageService != nil && c.Request().MultipartForm != nil {
for _, fieldName := range []string{"images", "image", "photo", "files"} {
files := c.Request().MultipartForm.File[fieldName]
for _, file := range files {
result, err := h.storageService.Upload(c.Request().Context(), file, "completions")
if err != nil {
return apperrors.BadRequest("error.failed_to_upload_image")
}
req.ImageURLs = append(req.ImageURLs, result.URL)
}
}
}
} else {
// Standard JSON request
if err := c.Bind(&req); err != nil { if err := c.Bind(&req); err != nil {
return apperrors.BadRequest("error.invalid_request") return apperrors.BadRequest("error.invalid_request")
} }
}
if err := c.Validate(&req); err != nil { if err := c.Validate(&req); err != nil {
return err return err
} }
-54
View File
@@ -44,60 +44,6 @@ func (h *UploadHandler) SetUploadService(s *services.UploadService) {
h.uploadService = s h.uploadService = s
} }
// UploadImage handles POST /api/uploads/image
// Accepts multipart/form-data with "file" field
func (h *UploadHandler) UploadImage(c echo.Context) error {
file, err := c.FormFile("file")
if err != nil {
return apperrors.BadRequest("error.no_file_provided")
}
// Get category from query param (default: images)
category := c.QueryParam("category")
if category == "" {
category = "images"
}
result, err := h.storageService.Upload(c.Request().Context(), file, category)
if err != nil {
return err
}
return c.JSON(http.StatusOK, result)
}
// UploadDocument handles POST /api/uploads/document
// Accepts multipart/form-data with "file" field
func (h *UploadHandler) UploadDocument(c echo.Context) error {
file, err := c.FormFile("file")
if err != nil {
return apperrors.BadRequest("error.no_file_provided")
}
result, err := h.storageService.Upload(c.Request().Context(), file, "documents")
if err != nil {
return err
}
return c.JSON(http.StatusOK, result)
}
// UploadCompletion handles POST /api/uploads/completion
// For task completion photos
func (h *UploadHandler) UploadCompletion(c echo.Context) error {
file, err := c.FormFile("file")
if err != nil {
return apperrors.BadRequest("error.no_file_provided")
}
result, err := h.storageService.Upload(c.Request().Context(), file, "completions")
if err != nil {
return err
}
return c.JSON(http.StatusOK, result)
}
// DeleteFileRequest is the request body for deleting a file. // DeleteFileRequest is the request body for deleting a file.
type DeleteFileRequest struct { type DeleteFileRequest struct {
URL string `json:"url" validate:"required"` URL string `json:"url" validate:"required"`
-3
View File
@@ -735,9 +735,6 @@ func setupUserRoutes(api *echo.Group, userHandler *handlers.UserHandler) {
func setupUploadRoutes(api *echo.Group, uploadHandler *handlers.UploadHandler) { func setupUploadRoutes(api *echo.Group, uploadHandler *handlers.UploadHandler) {
uploads := api.Group("/uploads") uploads := api.Group("/uploads")
{ {
uploads.POST("/image/", uploadHandler.UploadImage)
uploads.POST("/document/", uploadHandler.UploadDocument)
uploads.POST("/completion/", uploadHandler.UploadCompletion)
uploads.POST("/presign/", uploadHandler.PresignUpload) uploads.POST("/presign/", uploadHandler.PresignUpload)
uploads.DELETE("/", uploadHandler.DeleteFile) uploads.DELETE("/", uploadHandler.DeleteFile)
} }
+1 -15
View File
@@ -204,21 +204,7 @@ func (s *DocumentService) CreateDocument(ctx context.Context, req *requests.Crea
return nil, apperrors.Internal(err) return nil, apperrors.Internal(err)
} }
// Legacy multipart path — already-uploaded URLs. // Presigned-URL path — claimed image uploads become DocumentImage rows.
for _, imageURL := range req.ImageURLs {
if imageURL != "" {
img := &models.DocumentImage{
DocumentID: document.ID,
ImageURL: imageURL,
}
if err := s.documentRepo.WithContext(ctx).CreateDocumentImage(img); err != nil {
// Log but don't fail the whole operation
continue
}
}
}
// New presigned path — claimed image uploads become DocumentImage rows.
// The document_file row (if any) was already lifted onto the document above. // The document_file row (if any) was already lifted onto the document above.
for i := range claimedUploads { for i := range claimedUploads {
pu := claimedUploads[i] pu := claimedUploads[i]
+6 -41
View File
@@ -70,26 +70,10 @@ func TestDocumentService_CreateDocument_DefaultType(t *testing.T) {
assert.Equal(t, models.DocumentTypeGeneral, resp.DocumentType) assert.Equal(t, models.DocumentTypeGeneral, resp.DocumentType)
} }
func TestDocumentService_CreateDocument_WithImages(t *testing.T) { // TestDocumentService_CreateDocument_WithImages was removed alongside the
db := testutil.SetupTestDB(t) // legacy ImageURLs field. The presigned-URL flow is exercised end-to-end
documentRepo := repositories.NewDocumentRepository(db) // in the integration tests; mocking B2 + a pending_uploads fixture for a
residenceRepo := repositories.NewResidenceRepository(db) // unit test was deemed not worth the complexity.
service := NewDocumentService(documentRepo, residenceRepo)
user := testutil.CreateTestUser(t, db, "owner", "owner@test.com", "Password123")
residence := testutil.CreateTestResidence(t, db, user.ID, "Test House")
req := &requests.CreateDocumentRequest{
ResidenceID: residence.ID,
Title: "Receipt with photos",
ImageURLs: []string{"https://example.com/img1.jpg", "https://example.com/img2.jpg"},
}
resp, err := service.CreateDocument(context.Background(), req, user.ID)
require.NoError(t, err)
assert.NotNil(t, resp)
assert.Equal(t, "Receipt with photos", resp.Title)
}
func TestDocumentService_CreateDocument_AccessDenied(t *testing.T) { func TestDocumentService_CreateDocument_AccessDenied(t *testing.T) {
db := testutil.SetupTestDB(t) db := testutil.SetupTestDB(t)
@@ -630,27 +614,8 @@ func TestDocumentService_ActivateDocument_AccessDenied(t *testing.T) {
testutil.AssertAppError(t, err, http.StatusForbidden, "error.document_access_denied") testutil.AssertAppError(t, err, http.StatusForbidden, "error.document_access_denied")
} }
// === CreateDocument — with empty image URL in array (should skip) === // TestDocumentService_CreateDocument_WithEmptyImageURL was removed alongside
// the legacy ImageURLs field — empty-URL filtering is no longer a code path.
func TestDocumentService_CreateDocument_WithEmptyImageURL(t *testing.T) {
db := testutil.SetupTestDB(t)
documentRepo := repositories.NewDocumentRepository(db)
residenceRepo := repositories.NewResidenceRepository(db)
service := NewDocumentService(documentRepo, residenceRepo)
user := testutil.CreateTestUser(t, db, "owner", "owner@test.com", "Password123")
residence := testutil.CreateTestResidence(t, db, user.ID, "Test House")
req := &requests.CreateDocumentRequest{
ResidenceID: residence.ID,
Title: "Doc with empty images",
ImageURLs: []string{"", "https://example.com/img.jpg", ""},
}
resp, err := service.CreateDocument(context.Background(), req, user.ID)
require.NoError(t, err)
assert.NotNil(t, resp)
}
// === UpdateDocument — all optional fields === // === UpdateDocument — all optional fields ===
+7 -27
View File
@@ -727,23 +727,8 @@ func (s *TaskService) CreateCompletion(ctx context.Context, req *requests.Create
if err := s.taskRepo.WithContext(ctx).UpdateTx(tx, task); err != nil { if err := s.taskRepo.WithContext(ctx).UpdateTx(tx, task); err != nil {
return err return err
} }
// B-07: Create images inside the same transaction as completion. // Create completion image rows from the claimed pending_uploads.
// Two sources contribute, both produce TaskCompletionImage rows: // Bytes already live in B2; we just record the FK + URL.
// 1. Legacy multipart path — client uploaded via the API and got
// back URLs in req.ImageURLs.
// 2. New presigned path — client uploaded direct to B2 and we
// claimed the pending_uploads rows above.
for _, imageURL := range req.ImageURLs {
if imageURL != "" {
img := &models.TaskCompletionImage{
CompletionID: completion.ID,
ImageURL: imageURL,
}
if err := tx.Create(img).Error; err != nil {
return fmt.Errorf("failed to create completion image: %w", err)
}
}
}
for i := range claimedUploads { for i := range claimedUploads {
pu := claimedUploads[i] pu := claimedUploads[i]
img := &models.TaskCompletionImage{ img := &models.TaskCompletionImage{
@@ -1131,16 +1116,11 @@ func (s *TaskService) UpdateCompletion(ctx context.Context, completionID, userID
return nil, apperrors.Internal(err) return nil, apperrors.Internal(err)
} }
// Add any new images // Image-add on update is unsupported in the new flow — clients should
for _, imageURL := range req.ImageURLs { // instead delete and recreate the completion if image attachments need
image := &models.TaskCompletionImage{ // to change after the fact. The presigned-URL path requires a single
CompletionID: completion.ID, // "create with attachments" handshake and there's no equivalent attach-
ImageURL: imageURL, // to-existing pathway today. Add one here when a UI feature requires it.
}
if err := s.taskRepo.WithContext(ctx).CreateCompletionImage(image); err != nil {
log.Error().Err(err).Uint("completion_id", completion.ID).Msg("Failed to create completion image during update")
}
}
// Reload to get full associations // Reload to get full associations
updated, err := s.taskRepo.WithContext(ctx).FindCompletionByID(completionID) updated, err := s.taskRepo.WithContext(ctx).FindCompletionByID(completionID)