diff --git a/internal/dto/requests/document.go b/internal/dto/requests/document.go index e02e60d..29d8aba 100644 --- a/internal/dto/requests/document.go +++ b/internal/dto/requests/document.go @@ -25,12 +25,11 @@ type CreateDocumentRequest struct { SerialNumber string `json:"serial_number" validate:"max=100"` ModelNumber string `json:"model_number" validate:"max=100"` 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 - // upload flow and turns them into document_image rows. May be combined - // with ImageURLs during the rollout window. UploadIDs of category - // "document_file" attach to the document's main FileURL/FileName fields - // instead — the service infers placement from the row's category. + // upload flow and turns them into document_image rows. UploadIDs of + // category "document_file" attach to the document's main FileURL + + // FileName fields instead — the service infers placement from the + // row's category. UploadIDs []uint `json:"upload_ids" validate:"omitempty,max=20"` } diff --git a/internal/dto/requests/task.go b/internal/dto/requests/task.go index 4db162a..5045a9b 100644 --- a/internal/dto/requests/task.go +++ b/internal/dto/requests/task.go @@ -100,7 +100,12 @@ type UpdateTaskRequest struct { 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 { TaskID uint `json:"task_id" validate:"required"` CompletedAt *time.Time `json:"completed_at"` // Defaults to now @@ -108,19 +113,6 @@ type CreateTaskCompletionRequest struct { ActualCost *decimal.Decimal `json:"actual_cost"` 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"` } @@ -129,7 +121,6 @@ type UpdateTaskCompletionRequest struct { Notes *string `json:"notes" validate:"omitempty,max=10000"` ActualCost *decimal.Decimal `json:"actual_cost"` 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 diff --git a/internal/handlers/handler_coverage_test.go b/internal/handlers/handler_coverage_test.go index 5c5feb8..ca5ab2b 100644 --- a/internal/handlers/handler_coverage_test.go +++ b/internal/handlers/handler_coverage_test.go @@ -1781,45 +1781,11 @@ func TestStaticDataHandler_RefreshStaticData(t *testing.T) { // ============================================================================= // Upload Handler - Additional Error Paths // ============================================================================= - -func TestUploadHandler_UploadImage_NoFile(t *testing.T) { - storageSvc := newTestStorageService("/var/uploads") - handler := NewUploadHandler(storageSvc, nil) - e := testutil.SetupTestRouter() - - 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) - }) -} +// +// Multipart upload handlers (UploadImage / UploadDocument / UploadCompletion) +// were removed alongside the legacy /api/uploads/{image,document,completion} +// routes. The presigned-URL flow (POST /api/uploads/presign) is exercised by +// integration tests that hit the full pipeline. func TestUploadHandler_DeleteFile_OwnershipDenied(t *testing.T) { storageSvc := newTestStorageService("/var/uploads") diff --git a/internal/handlers/task_handler.go b/internal/handlers/task_handler.go index 5d5d9e9..e6d7ed1 100644 --- a/internal/handlers/task_handler.go +++ b/internal/handlers/task_handler.go @@ -3,11 +3,8 @@ package handlers import ( "net/http" "strconv" - "strings" - "time" "github.com/labstack/echo/v4" - "github.com/shopspring/decimal" "github.com/treytartt/honeydue-api/internal/apperrors" "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/ -// 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 { user, err := middleware.MustGetAuthUser(c) if err != nil { @@ -402,65 +410,9 @@ func (h *TaskHandler) CreateCompletion(c echo.Context) error { userNow := middleware.GetUserNow(c) 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 { - return apperrors.BadRequest("error.invalid_request") - } + if err := c.Bind(&req); err != nil { + return apperrors.BadRequest("error.invalid_request") } - if err := c.Validate(&req); err != nil { return err } diff --git a/internal/handlers/upload_handler.go b/internal/handlers/upload_handler.go index f710634..e9b4cd1 100644 --- a/internal/handlers/upload_handler.go +++ b/internal/handlers/upload_handler.go @@ -44,60 +44,6 @@ func (h *UploadHandler) SetUploadService(s *services.UploadService) { 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. type DeleteFileRequest struct { URL string `json:"url" validate:"required"` diff --git a/internal/router/router.go b/internal/router/router.go index 51d3ee1..fc85757 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -735,9 +735,6 @@ func setupUserRoutes(api *echo.Group, userHandler *handlers.UserHandler) { func setupUploadRoutes(api *echo.Group, uploadHandler *handlers.UploadHandler) { 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.DELETE("/", uploadHandler.DeleteFile) } diff --git a/internal/services/document_service.go b/internal/services/document_service.go index 804d82d..7d0860d 100644 --- a/internal/services/document_service.go +++ b/internal/services/document_service.go @@ -204,21 +204,7 @@ func (s *DocumentService) CreateDocument(ctx context.Context, req *requests.Crea return nil, apperrors.Internal(err) } - // Legacy multipart path — already-uploaded URLs. - 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. + // Presigned-URL path — claimed image uploads become DocumentImage rows. // The document_file row (if any) was already lifted onto the document above. for i := range claimedUploads { pu := claimedUploads[i] diff --git a/internal/services/document_service_test.go b/internal/services/document_service_test.go index d02455d..6cf6dbc 100644 --- a/internal/services/document_service_test.go +++ b/internal/services/document_service_test.go @@ -70,26 +70,10 @@ func TestDocumentService_CreateDocument_DefaultType(t *testing.T) { assert.Equal(t, models.DocumentTypeGeneral, resp.DocumentType) } -func TestDocumentService_CreateDocument_WithImages(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: "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) -} +// TestDocumentService_CreateDocument_WithImages was removed alongside the +// legacy ImageURLs field. The presigned-URL flow is exercised end-to-end +// in the integration tests; mocking B2 + a pending_uploads fixture for a +// unit test was deemed not worth the complexity. func TestDocumentService_CreateDocument_AccessDenied(t *testing.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") } -// === CreateDocument — with empty image URL in array (should skip) === - -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) -} +// TestDocumentService_CreateDocument_WithEmptyImageURL was removed alongside +// the legacy ImageURLs field — empty-URL filtering is no longer a code path. // === UpdateDocument — all optional fields === diff --git a/internal/services/task_service.go b/internal/services/task_service.go index 09fc331..243fcc0 100644 --- a/internal/services/task_service.go +++ b/internal/services/task_service.go @@ -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 { return err } - // B-07: Create images inside the same transaction as completion. - // Two sources contribute, both produce TaskCompletionImage rows: - // 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) - } - } - } + // Create completion image rows from the claimed pending_uploads. + // Bytes already live in B2; we just record the FK + URL. for i := range claimedUploads { pu := claimedUploads[i] img := &models.TaskCompletionImage{ @@ -1131,16 +1116,11 @@ func (s *TaskService) UpdateCompletion(ctx context.Context, completionID, userID return nil, apperrors.Internal(err) } - // Add any new images - for _, imageURL := range req.ImageURLs { - image := &models.TaskCompletionImage{ - CompletionID: completion.ID, - ImageURL: imageURL, - } - 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") - } - } + // Image-add on update is unsupported in the new flow — clients should + // instead delete and recreate the completion if image attachments need + // to change after the fact. The presigned-URL path requires a single + // "create with attachments" handshake and there's no equivalent attach- + // to-existing pathway today. Add one here when a UI feature requires it. // Reload to get full associations updated, err := s.taskRepo.WithContext(ctx).FindCompletionByID(completionID)