From 51093cbd0749c82dc559cc0a95a692628d321181 Mon Sep 17 00:00:00 2001 From: nic Date: Mon, 5 May 2025 12:44:07 -0400 Subject: [PATCH] feat: added multi-upload field for files instead of continually adding single new fields --- apps/web/main.go | 2 - internal/handlers/web/documents.go | 276 +++++++++++------------- templates/partials/document_upload.html | 58 +++-- 3 files changed, 145 insertions(+), 191 deletions(-) diff --git a/apps/web/main.go b/apps/web/main.go index db33232..120e538 100644 --- a/apps/web/main.go +++ b/apps/web/main.go @@ -63,8 +63,6 @@ func main() { protected.HandleFunc("/documents", web.DocumentsHandler).Methods("GET") protected.HandleFunc("/process-csv", web.ProcessCSVHandler).Methods("POST") protected.HandleFunc("/upload-documents", web.UploadDocumentsHandler).Methods("POST") - protected.HandleFunc("/document-field-add", web.DocumentFieldAddHandler).Methods("GET") - protected.HandleFunc("/document-field-remove", web.DocumentFieldRemoveHandler).Methods("GET") // Document removal routes protected.HandleFunc("/documents/remove", web.DocumentRemoveHandler).Methods("GET") diff --git a/internal/handlers/web/documents.go b/internal/handlers/web/documents.go index 4a612ef..93bc7f1 100644 --- a/internal/handlers/web/documents.go +++ b/internal/handlers/web/documents.go @@ -190,16 +190,14 @@ func UploadDocumentsHandler(w http.ResponseWriter, r *http.Request) { http.Error(w, "Unable to parse form: "+err.Error(), http.StatusBadRequest) return } + defer r.MultipartForm.RemoveAll() // Clean up temporary files // Get job numbers from form values jobNumbers := r.FormValue("jobNumbers") if jobNumbers == "" { - jobNumbers = r.FormValue("job-ids") - if jobNumbers == "" { - log.Printf("No job numbers found in form values") - http.Error(w, "No job numbers provided", http.StatusBadRequest) - return - } + log.Printf("No job numbers found in hidden 'jobNumbers' input.") + http.Error(w, "No job numbers provided", http.StatusBadRequest) + return } log.Printf("Job numbers: %s", jobNumbers) @@ -209,67 +207,51 @@ func UploadDocumentsHandler(w http.ResponseWriter, r *http.Request) { return } + // Get the single document type + docType := r.FormValue("documentType") + if docType == "" { + log.Printf("No document type selected") + http.Error(w, "Please select a document type", http.StatusBadRequest) + return + } + log.Printf("Document Type selected: %s", docType) + + // Get the uploaded files from the 'documentFiles' input + fileHeaders := r.MultipartForm.File["documentFiles"] + if len(fileHeaders) == 0 { + http.Error(w, "No documents selected for upload", http.StatusBadRequest) + return + } + // Store file metadata type FileMetadata struct { - FormField string - FileName string - Type string - CustomName string - TempFile string // Path to temp file if we create one - FileData []byte // File data if small enough to keep in memory - File *os.File // Open file handle if we're using a temp file + FileName string + Type string + TempFile string // Path to temp file + File *os.File // Open file handle for the temp file } var filesToUpload []FileMetadata - // Process each file upload in the form - for formField, fileHeaders := range r.MultipartForm.File { - if !strings.HasPrefix(formField, "document-file-") { - continue - } - - if len(fileHeaders) == 0 { - continue - } - - fileHeader := fileHeaders[0] + // Process each uploaded file + for _, fileHeader := range fileHeaders { if fileHeader.Filename == "" { + log.Printf("Skipping file header with empty filename.") continue } - // Get suffix for related form fields - suffix := strings.TrimPrefix(formField, "document-file-") - typeField := "document-type-" + suffix - nameField := "document-name-" + suffix - - // Get document type and custom name - docType := r.FormValue(typeField) - if docType == "" { - log.Printf("No document type for file %s, skipping", fileHeader.Filename) - continue - } - - customName := r.FormValue(nameField) - if customName != "" && !strings.Contains(customName, ".") { - ext := filepath.Ext(fileHeader.Filename) - if ext != "" { - customName = customName + ext - } - } - // Open the uploaded file uploadedFile, err := fileHeader.Open() if err != nil { log.Printf("Error opening uploaded file %s: %v", fileHeader.Filename, err) - continue + // Optionally: decide if one error should halt all uploads or just skip this file + continue // Skip this file } // Prepare metadata metadata := FileMetadata{ - FormField: formField, - FileName: fileHeader.Filename, - Type: docType, - CustomName: customName, + FileName: fileHeader.Filename, + Type: docType, // Use the single document type for all files } // Create a temp file for the upload (regardless of size to ensure streaming) @@ -277,32 +259,55 @@ func UploadDocumentsHandler(w http.ResponseWriter, r *http.Request) { if err != nil { log.Printf("Error creating temp file for %s: %v", fileHeader.Filename, err) uploadedFile.Close() - continue + continue // Skip this file } // Copy the file content to the temp file bytesCopied, err := io.Copy(tempFile, uploadedFile) - uploadedFile.Close() + uploadedFile.Close() // Close the original multipart file handle if err != nil { log.Printf("Error copying to temp file for %s: %v", fileHeader.Filename, err) - tempFile.Close() - os.Remove(tempFile.Name()) - continue + tempFile.Close() // Close the temp file handle + os.Remove(tempFile.Name()) // Remove the partially written temp file + continue // Skip this file } log.Printf("Copied %d bytes of %s to temporary file: %s", bytesCopied, fileHeader.Filename, tempFile.Name()) - // Seek back to beginning for later reading - tempFile.Seek(0, 0) + // Seek back to beginning for later reading by upload goroutines + if _, err := tempFile.Seek(0, 0); err != nil { + log.Printf("Error seeking temp file for %s: %v", fileHeader.Filename, err) + tempFile.Close() + os.Remove(tempFile.Name()) + continue // Skip this file + } metadata.TempFile = tempFile.Name() - metadata.File = tempFile // Store the open file handle + metadata.File = tempFile // Store the open temp file handle filesToUpload = append(filesToUpload, metadata) } + // Ensure temp files associated with metadata are closed and removed later + defer func() { + log.Println("Running deferred cleanup for temp files...") + for _, fm := range filesToUpload { + if fm.File != nil { + fm.File.Close() + if fm.TempFile != "" { + err := os.Remove(fm.TempFile) + if err != nil && !os.IsNotExist(err) { // Don't log error if file already gone + log.Printf("Error cleaning up temp file %s: %v", fm.TempFile, err) + } else if err == nil { + log.Printf("Cleaned up temp file: %s", fm.TempFile) + } + } + } + } + }() + if len(filesToUpload) == 0 { - http.Error(w, "No valid documents selected for upload", http.StatusBadRequest) + http.Error(w, "No valid documents could be processed for upload", http.StatusBadRequest) return } @@ -333,15 +338,19 @@ func UploadDocumentsHandler(w http.ResponseWriter, r *http.Request) { semaphore := make(chan struct{}, maxConcurrent) // Start the upload workers - log.Printf("Starting %d upload workers for %d total uploads", - maxConcurrent, totalUploads) + log.Printf("Starting %d upload workers for %d total uploads (%d jobs x %d files)", + maxConcurrent, totalUploads, len(jobs), len(filesToUpload)) for _, jobID := range jobs { for _, metadata := range filesToUpload { + // Create a closure capture of the metadata for the goroutine + // This is crucial because the 'metadata' variable in the loop will change + currentMetadata := metadata + wg.Add(1) // Launch a goroutine for each job+document combination - go func(jobID string, metadata FileMetadata) { + go func(jobID string, meta FileMetadata) { defer wg.Done() // Acquire a semaphore slot @@ -351,16 +360,14 @@ func UploadDocumentsHandler(w http.ResponseWriter, r *http.Request) { // Add a small delay to avoid overwhelming the API time.Sleep(requestDelay) - // Get the file name to use (custom name or original) - fileName := metadata.FileName - if metadata.CustomName != "" { - fileName = metadata.CustomName - } + // Get the file name to use (original filename) + fileName := meta.FileName - // Create a fresh file reader for each upload to avoid sharing file handles - fileHandle, err := os.Open(metadata.TempFile) + // Use a fresh reader for the temp file for each upload goroutine + // Re-open the temp file for reading to avoid race conditions on the file pointer + fileHandle, err := os.Open(meta.TempFile) if err != nil { - log.Printf("Error opening temp file for %s: %v", fileName, err) + log.Printf("Error re-opening temp file %s for job %s: %v", meta.TempFile, jobID, err) resultsChan <- UploadResult{ JobID: jobID, DocName: fileName, @@ -373,10 +380,13 @@ func UploadDocumentsHandler(w http.ResponseWriter, r *http.Request) { defer fileHandle.Close() // Close this handle when done with this upload // Get the expected file size for validation - fileInfo, statErr := os.Stat(metadata.TempFile) + fileInfo, statErr := fileHandle.Stat() // Stat the newly opened handle var expectedSize int64 if statErr == nil { expectedSize = fileInfo.Size() + } else { + log.Printf("Error getting file info for %s (job %s): %v", fileName, jobID, statErr) + // Continue without size check if stat fails, but log it } // Add jitter delay for large batch uploads (more than 10 jobs) @@ -387,14 +397,12 @@ func UploadDocumentsHandler(w http.ResponseWriter, r *http.Request) { // Wrap with size tracker sizeTracker := &readCloserWithSize{reader: fileHandle, size: 0} - fileReader := sizeTracker - // Log streaming progress - log.Printf("Starting to stream file %s to job %s from fresh file handle", fileName, jobID) + log.Printf("Starting to stream file %s to job %s from temp file %s", fileName, jobID, meta.TempFile) // Call ServiceTrade API with the file reader uploadStart := time.Now() - result, err := session.UploadAttachmentFile(jobID, fileName, metadata.Type, fileReader) + result, err := session.UploadAttachmentFile(jobID, fileName, meta.Type, sizeTracker) uploadDuration := time.Since(uploadStart) // Get the actual size that was uploaded @@ -402,7 +410,7 @@ func UploadDocumentsHandler(w http.ResponseWriter, r *http.Request) { // Verify the upload size matches the expected file size sizeMatch := true - if expectedSize > 0 && math.Abs(float64(expectedSize-fileSize)) > float64(expectedSize)*0.05 { + if expectedSize > 0 && math.Abs(float64(expectedSize-fileSize)) > float64(expectedSize)*0.05 { // Allow 5% tolerance sizeMatch = false log.Printf("WARNING: Size mismatch for %s to job %s. Expected: %d, Uploaded: %d", fileName, jobID, expectedSize, fileSize) @@ -440,27 +448,17 @@ func UploadDocumentsHandler(w http.ResponseWriter, r *http.Request) { FileSize: fileSize, } } - }(jobID, metadata) + }(jobID, currentMetadata) // Pass the captured metadata } } - // Clean up temp files when all uploads are done - defer func() { - for _, metadata := range filesToUpload { - if metadata.File != nil { - metadata.File.Close() - if metadata.TempFile != "" { - os.Remove(metadata.TempFile) - log.Printf("Cleaned up temp file: %s", metadata.TempFile) - } - } - } - }() + // NOTE: The deferred cleanup function for temp files defined earlier will run after this point. // Close the results channel when all uploads are done go func() { wg.Wait() close(resultsChan) + log.Println("All upload goroutines finished.") }() // Collect results @@ -527,15 +525,18 @@ func UploadDocumentsHandler(w http.ResponseWriter, r *http.Request) { // File count stat resultHTML.WriteString("
") - resultHTML.WriteString(fmt.Sprintf("
%d
", totalSuccess+totalFailure)) + // Use resultsCount which reflects total files attempted + resultHTML.WriteString(fmt.Sprintf("
%d
", resultsCount)) resultHTML.WriteString("
Files Processed
") resultHTML.WriteString("
") resultHTML.WriteString("") // End of upload-stats // Add completion message - if totalFailure == 0 { + if totalFailure == 0 && resultsCount > 0 { resultHTML.WriteString("

All documents were successfully uploaded to ServiceTrade!

") + } else if resultsCount == 0 { + resultHTML.WriteString("

No documents were processed for upload.

") } else { resultHTML.WriteString("

Some documents failed to upload. See details below.

") } @@ -555,28 +556,37 @@ func UploadDocumentsHandler(w http.ResponseWriter, r *http.Request) { for _, jobID := range sortedJobs { jobResults := results[jobID] - // Determine job success status - jobSuccess := true + // Determine job success status based on results for *this job* + jobHasSuccess := false + jobHasFailure := false for _, result := range jobResults { - if !result.Success { - jobSuccess = false - break + if result.Success { + jobHasSuccess = true + } else { + jobHasFailure = true } } - // Job result row - jobClass := "success" - if !jobSuccess { - jobClass = "error" + // Job result row styling + jobClass := "neutral" // Default if somehow no results for a job ID + if jobHasSuccess && !jobHasFailure { + jobClass = "success" + } else if jobHasFailure { + jobClass = "error" // Prioritize showing error if any file failed for this job } resultHTML.WriteString(fmt.Sprintf("
", jobClass)) - resultHTML.WriteString(fmt.Sprintf("Job ID: %s", jobID)) + resultHTML.WriteString(fmt.Sprintf("
Job ID: %s
", jobID)) // Wrap ID for better styling // File results if len(jobResults) > 0 { resultHTML.WriteString("
") + // Sort file results by name for consistency + sort.Slice(jobResults, func(i, j int) bool { + return jobResults[i].DocName < jobResults[j].DocName + }) + for _, result := range jobResults { fileClass := "success" icon := "✓" @@ -585,7 +595,9 @@ func UploadDocumentsHandler(w http.ResponseWriter, r *http.Request) { if !result.Success { fileClass = "error" icon = "✗" - message = result.Error + // Sanitize error message slightly for HTML display if needed + message = strings.ReplaceAll(result.Error, "<", "<") + message = strings.ReplaceAll(message, ">", ">") } resultHTML.WriteString(fmt.Sprintf("
", fileClass)) @@ -597,7 +609,7 @@ func UploadDocumentsHandler(w http.ResponseWriter, r *http.Request) { resultHTML.WriteString("
") // End of file-results } else { - resultHTML.WriteString("

No files processed for this job.

") + resultHTML.WriteString("

No file upload results for this job.

") // More specific message } resultHTML.WriteString("
") // End of job-result @@ -622,7 +634,10 @@ func (r *readCloserWithSize) Read(p []byte) (n int, err error) { } func (r *readCloserWithSize) Close() error { - return r.reader.Close() + if r.reader != nil { + return r.reader.Close() + } + return nil // Allow closing nil reader safely } // Size returns the current size of data read @@ -631,58 +646,7 @@ func (r *readCloserWithSize) Size() int64 { } // DocumentFieldAddHandler generates a new document field for the form -func DocumentFieldAddHandler(w http.ResponseWriter, r *http.Request) { - // Generate a random ID for the new field - newId := fmt.Sprintf("%d", time.Now().UnixNano()) - - // Create HTML for a new document row - html := fmt.Sprintf(` -
-
- - -
- -
-
- - -
- -
- - -
-
- - -
- `, newId, newId, newId, newId, newId, newId, newId, newId, newId) - - w.Header().Set("Content-Type", "text/html") - w.Write([]byte(html)) -} +// REMOVED as it's no longer needed with the multi-file input // DocumentFieldRemoveHandler handles the removal of a document field -func DocumentFieldRemoveHandler(w http.ResponseWriter, r *http.Request) { - // We read the ID but don't need to use it for simple removal - _ = r.URL.Query().Get("id") - - // Count how many document rows exist - // For simplicity, we'll just return an empty response to remove the field - // In a complete implementation, we'd check if this is the last field and handle that case - - w.Header().Set("Content-Type", "text/html") - w.Write([]byte("")) -} +// REMOVED as it's no longer needed with the multi-file input diff --git a/templates/partials/document_upload.html b/templates/partials/document_upload.html index 8aed0c1..da66847 100644 --- a/templates/partials/document_upload.html +++ b/templates/partials/document_upload.html @@ -49,40 +49,29 @@