diff --git a/internal/inquiry/errors.go b/internal/inquiry/errors.go index 4c0dc65..b6db045 100644 --- a/internal/inquiry/errors.go +++ b/internal/inquiry/errors.go @@ -1,6 +1,10 @@ package inquiry -import "errors" +import ( + "errors" + "fmt" + "strings" +) // Inquiry-specific errors var ( @@ -25,6 +29,34 @@ func (e *DuplicateInquiryError) Error() string { } // NewDuplicateInquiryError creates a new duplicate inquiry error +// InvalidStatusTransitionError is returned when an inquiry status change is not allowed. +type InvalidStatusTransitionError struct { + CurrentStatus string + NewStatus string + Allowed []string +} + +func (e *InvalidStatusTransitionError) Error() string { + if len(e.Allowed) == 0 { + return fmt.Sprintf("Inquiry status is already %s and cannot be changed", e.CurrentStatus) + } + return fmt.Sprintf( + "Cannot change inquiry status from %s to %s. Allowed next statuses: %s", + e.CurrentStatus, + e.NewStatus, + strings.Join(e.Allowed, ", "), + ) +} + +// NewInvalidStatusTransitionError creates a status transition error with allowed next statuses. +func NewInvalidStatusTransitionError(currentStatus, newStatus string, allowed []string) *InvalidStatusTransitionError { + return &InvalidStatusTransitionError{ + CurrentStatus: currentStatus, + NewStatus: newStatus, + Allowed: allowed, + } +} + func NewDuplicateInquiryError(inquiryType, existingID, status string, createdAt int64) *DuplicateInquiryError { var message string if inquiryType == "email" { diff --git a/internal/inquiry/form.go b/internal/inquiry/form.go index 391721b..7f87f2c 100644 --- a/internal/inquiry/form.go +++ b/internal/inquiry/form.go @@ -1,6 +1,7 @@ package inquiry import ( + "strings" "tm/pkg/response" "tm/pkg/security" @@ -21,7 +22,7 @@ type CreateInquiryForm struct { // UpdateInquiryStatusForm represents the data required to update inquiry status type UpdateInquiryStatusForm struct { Status string `json:"status" valid:"required,in(pending|reviewed|approved|rejected)" example:"reviewed"` // New status - Reason string `json:"reason" valid:"required,length(5|500)" example:"Initial review completed"` // Reason for status change + Reason string `json:"reason" valid:"optional,length(1|500)" example:"Initial review completed"` // Reason for status change Description string `json:"description" valid:"optional,length(0|1000)" example:"Additional notes about the review"` // Optional description } @@ -125,6 +126,11 @@ func (f *CreateInquiryForm) ValidateAndSanitize(xssPolicy *security.XSSPolicy) e // ValidateAndSanitize validates and sanitizes the status update form func (f *UpdateInquiryStatusForm) ValidateAndSanitize(xssPolicy *security.XSSPolicy) error { + f.Status = strings.ToLower(strings.TrimSpace(f.Status)) + if strings.TrimSpace(f.Reason) == "" { + f.Reason = "Status updated to " + f.Status + } + var err error // Sanitize Reason diff --git a/internal/inquiry/form_validation_test.go b/internal/inquiry/form_validation_test.go new file mode 100644 index 0000000..a6f4b70 --- /dev/null +++ b/internal/inquiry/form_validation_test.go @@ -0,0 +1,35 @@ +package inquiry + +import ( + "encoding/json" + "testing" + + "tm/pkg/security" +) + +func TestUpdateInquiryStatusFormValidation(t *testing.T) { + xssPolicy := security.NewXSSPolicy() + + cases := []struct { + name string + body string + wantErr bool + }{ + {"status only", `{"status":"reviewed"}`, false}, + {"capitalized status", `{"status":"Reviewed"}`, false}, + {"valid with reason", `{"status":"reviewed","reason":"Initial review completed"}`, false}, + {"user payload", `{"status":"reviewed","reason":"test","description":"test"}`, false}, + {"invalid status", `{"status":"done"}`, true}, + } + for _, tc := range cases { + var form UpdateInquiryStatusForm + if err := json.Unmarshal([]byte(tc.body), &form); err != nil { + t.Fatalf("%s: unmarshal failed: %v", tc.name, err) + } + + err := form.ValidateAndSanitize(xssPolicy) + if (err != nil) != tc.wantErr { + t.Errorf("%s: got err=%v wantErr=%v", tc.name, err, tc.wantErr) + } + } +} diff --git a/internal/inquiry/handler.go b/internal/inquiry/handler.go index 278a241..3125e88 100644 --- a/internal/inquiry/handler.go +++ b/internal/inquiry/handler.go @@ -42,14 +42,13 @@ func NewHandler(service Service, logger logger.Logger, hcaptchaVerifier hcaptcha // @Failure 500 {object} response.APIResponse "Internal server error" // @Router /api/v1/inquiries [post] func (h *Handler) CreateInquiry(c echo.Context) error { - form, err := response.Parse[CreateInquiryForm](c) - if err != nil { - return response.BadRequest(c, "Invalid request format", "") + var form CreateInquiryForm + if err := c.Bind(&form); err != nil { + return handleBindError(c, err) } - // Validate and sanitize form if err := form.ValidateAndSanitize(h.xssPolicy); err != nil { - return response.ValidationError(c, err.Error(), "") + return handleValidationError(c, err) } // Verify hCaptcha token @@ -64,7 +63,7 @@ func (h *Handler) CreateInquiry(c echo.Context) error { } // Call service - inquiry, err := h.service.Create(c.Request().Context(), form) + inquiry, err := h.service.Create(c.Request().Context(), &form) if err != nil { // Check if it's a duplicate inquiry error if duplicateErr, ok := err.(*DuplicateInquiryError); ok { @@ -110,7 +109,7 @@ func (h *Handler) GetInquiryByID(c echo.Context) error { inquiry, err := h.service.GetByID(c.Request().Context(), idStr) if err != nil { - return response.NotFound(c, "Inquiry not found") + return handleServiceError(c, err, "Failed to retrieve inquiry") } return response.Success(c, inquiry, "Inquiry retrieved successfully") @@ -133,14 +132,13 @@ func (h *Handler) GetInquiryByID(c echo.Context) error { func (h *Handler) UpdateInquiryStatus(c echo.Context) error { idStr := c.Param("id") - form, err := response.Parse[UpdateInquiryStatusForm](c) - if err != nil { - return response.BadRequest(c, "Invalid request format", "") + var form UpdateInquiryStatusForm + if err := c.Bind(&form); err != nil { + return handleBindError(c, err) } - // Validate and sanitize form if err := form.ValidateAndSanitize(h.xssPolicy); err != nil { - return response.ValidationError(c, err.Error(), "") + return handleValidationError(c, err) } // Get user ID from context (assuming it's set by auth middleware) @@ -148,9 +146,9 @@ func (h *Handler) UpdateInquiryStatus(c echo.Context) error { // changedBy := c.Get("user_id").(string) // Call service - inquiry, err := h.service.UpdateStatus(c.Request().Context(), idStr, form, changedBy) + inquiry, err := h.service.UpdateStatus(c.Request().Context(), idStr, &form, changedBy) if err != nil { - return response.BadRequest(c, err.Error(), "") + return handleServiceError(c, err, "Failed to update inquiry status") } return response.Success(c, inquiry, "Inquiry status updated successfully") @@ -174,14 +172,13 @@ func (h *Handler) UpdateInquiryStatus(c echo.Context) error { // @Failure 500 {object} response.APIResponse "Internal server error" // @Router /admin/v1/inquiries [get] func (h *Handler) SearchInquiries(c echo.Context) error { - form, err := response.Parse[SearchInquiriesForm](c) - if err != nil { - return response.BadRequest(c, "Invalid request format", "") + var form SearchInquiriesForm + if err := c.Bind(&form); err != nil { + return handleBindError(c, err) } - // Validate and sanitize form if err := form.ValidateAndSanitize(h.xssPolicy); err != nil { - return response.ValidationError(c, err.Error(), "") + return handleValidationError(c, err) } // Call service @@ -190,7 +187,7 @@ func (h *Handler) SearchInquiries(c echo.Context) error { return response.PaginationBadRequest(c, err) } - inquiries, err := h.service.Search(c.Request().Context(), form, pagination) + inquiries, err := h.service.Search(c.Request().Context(), &form, pagination) if err != nil { if response.IsListPaginationError(err) { return response.PaginationBadRequest(c, err) @@ -220,7 +217,7 @@ func (h *Handler) DeleteInquiry(c echo.Context) error { err := h.service.Delete(c.Request().Context(), id) if err != nil { - return response.BadRequest(c, err.Error(), "") + return handleServiceError(c, err, "Failed to delete inquiry") } return response.Success(c, map[string]interface{}{ diff --git a/internal/inquiry/handler_errors.go b/internal/inquiry/handler_errors.go new file mode 100644 index 0000000..2bd5ac1 --- /dev/null +++ b/internal/inquiry/handler_errors.go @@ -0,0 +1,38 @@ +package inquiry + +import ( + "errors" + "strings" + + "tm/pkg/response" + + "github.com/labstack/echo/v4" +) + +func handleBindError(c echo.Context, err error) error { + return response.BadRequest(c, "Invalid request format", err.Error()) +} + +func handleValidationError(c echo.Context, err error) error { + return response.ValidationError(c, "Please fix the validation errors", FormatValidationErrors(err)) +} + +func handleServiceError(c echo.Context, err error, fallback string) error { + if err == nil { + return nil + } + if errors.Is(err, ErrInquiryNotFound) { + return response.NotFound(c, "Inquiry not found") + } + + var transitionErr *InvalidStatusTransitionError + if errors.As(err, &transitionErr) { + return response.BadRequest(c, transitionErr.Error(), "") + } + + if strings.Contains(err.Error(), "cannot be updated") { + return response.BadRequest(c, err.Error(), "") + } + + return response.InternalServerError(c, fallback) +} diff --git a/internal/inquiry/repository.go b/internal/inquiry/repository.go index 9bda555..b6b5629 100644 --- a/internal/inquiry/repository.go +++ b/internal/inquiry/repository.go @@ -105,7 +105,7 @@ func (r *inquiryRepository) GetByID(ctx context.Context, id string) (*Inquiry, e inquiry, err := r.ormRepo.FindByID(ctx, id) if err != nil { if errors.Is(err, orm.ErrDocumentNotFound) { - return nil, errors.New("inquiry not found") + return nil, ErrInquiryNotFound } r.logger.Error("Failed to get inquiry by ID", map[string]interface{}{ "error": err.Error(), diff --git a/internal/inquiry/service.go b/internal/inquiry/service.go index 937b419..34d9ae3 100644 --- a/internal/inquiry/service.go +++ b/internal/inquiry/service.go @@ -251,17 +251,16 @@ func (s *inquiryService) validateStatusTransition(ctx context.Context, id string allowedNextStatuses, exists := allowedTransitions[currentStatus] if !exists { - return errors.New("invalid current status") + return fmt.Errorf("inquiry has unknown status %q and cannot be updated", currentStatus) } - // Check if transition is allowed for _, allowedStatus := range allowedNextStatuses { if allowedStatus == newStatus { return nil } } - return errors.New("invalid status transition: cannot change from " + currentStatus + " to " + newStatus) + return NewInvalidStatusTransitionError(currentStatus, newStatus, allowedNextStatuses) } // Search searches inquiries with search and filters diff --git a/internal/inquiry/validation.go b/internal/inquiry/validation.go new file mode 100644 index 0000000..f2679c1 --- /dev/null +++ b/internal/inquiry/validation.go @@ -0,0 +1,120 @@ +package inquiry + +import ( + "fmt" + "regexp" + "strings" + + "github.com/asaskevich/govalidator" +) + +var ( + lengthRulePattern = regexp.MustCompile(`length\((\d+)\|(\d+)\)`) + inRulePattern = regexp.MustCompile(`in\(([^)]+)\)`) +) + +// FormatValidationErrors converts govalidator errors into user-facing messages. +func FormatValidationErrors(err error) string { + if err == nil { + return "" + } + + errs, ok := err.(govalidator.Errors) + if !ok { + return humanizeValidationError(err.Error()) + } + + messages := make([]string, 0, len(errs)) + for _, item := range errs { + messages = append(messages, humanizeValidationError(item.Error())) + } + return strings.Join(messages, "; ") +} + +func humanizeValidationError(raw string) string { + field, rule, ok := parseGovalidatorError(raw) + if !ok { + return raw + } + + label := inquiryFieldLabel(field) + lowerRule := strings.ToLower(rule) + + switch { + case strings.Contains(lowerRule, "required"): + return label + " is required" + case strings.Contains(lowerRule, "email"): + return label + " must be a valid email address" + case strings.Contains(lowerRule, "length("): + if min, max, ok := parseLengthRule(rule); ok { + if min == max { + return fmt.Sprintf("%s must be exactly %d characters", label, min) + } + return fmt.Sprintf("%s must be between %d and %d characters", label, min, max) + } + return label + " has an invalid length" + case strings.Contains(lowerRule, "in("): + if values, ok := parseInRule(rule); ok { + return fmt.Sprintf("%s must be one of: %s", label, strings.Join(values, ", ")) + } + return label + " has an invalid value" + case strings.Contains(lowerRule, "range("): + return label + " is out of the allowed range" + default: + return label + " is invalid" + } +} + +func parseGovalidatorError(raw string) (field, rule string, ok bool) { + parts := strings.SplitN(raw, ": ", 2) + if len(parts) != 2 { + return "", "", false + } + return parts[0], parts[1], true +} + +func parseLengthRule(rule string) (min, max int, ok bool) { + matches := lengthRulePattern.FindStringSubmatch(rule) + if len(matches) != 3 { + return 0, 0, false + } + _, err := fmt.Sscanf(matches[1]+" "+matches[2], "%d %d", &min, &max) + return min, max, err == nil +} + +func parseInRule(rule string) (values []string, ok bool) { + matches := inRulePattern.FindStringSubmatch(rule) + if len(matches) != 2 { + return nil, false + } + for _, value := range strings.Split(matches[1], "|") { + values = append(values, value) + } + return values, len(values) > 0 +} + +func inquiryFieldLabel(field string) string { + field = strings.TrimPrefix(field, "CreateInquiryForm.") + field = strings.TrimPrefix(field, "UpdateInquiryStatusForm.") + field = strings.TrimPrefix(field, "SearchInquiriesForm.") + + labels := map[string]string{ + "FullName": "Full name", + "CompanyName": "Company name", + "WorkEmail": "Work email", + "PhoneNumber": "Phone number", + "HCaptchaToken": "hCaptcha token", + "Status": "Status", + "Reason": "Reason", + "Description": "Description", + "Search": "Search query", + "Limit": "Limit", + "Offset": "Offset", + "SortBy": "Sort field", + "SortOrder": "Sort order", + } + if label, exists := labels[field]; exists { + return label + } + return field +} diff --git a/internal/inquiry/validation_test.go b/internal/inquiry/validation_test.go new file mode 100644 index 0000000..38d2712 --- /dev/null +++ b/internal/inquiry/validation_test.go @@ -0,0 +1,39 @@ +package inquiry + +import ( + "testing" + + "github.com/asaskevich/govalidator" +) + +func TestFormatValidationErrors(t *testing.T) { + form := UpdateInquiryStatusForm{ + Status: "done", + Reason: "", + } + _, err := govalidator.ValidateStruct(form) + if err == nil { + t.Fatal("expected validation error") + } + + message := FormatValidationErrors(err) + if message == "" { + t.Fatal("expected formatted validation message") + } + if message == err.Error() { + t.Fatalf("expected humanized message, got raw error: %s", message) + } +} + +func TestInvalidStatusTransitionErrorMessage(t *testing.T) { + err := NewInvalidStatusTransitionError(StatusPending, StatusApproved, []string{StatusReviewed, StatusRejected}) + expected := "Cannot change inquiry status from pending to approved. Allowed next statuses: reviewed, rejected" + if err.Error() != expected { + t.Fatalf("got %q, want %q", err.Error(), expected) + } + + finalErr := NewInvalidStatusTransitionError(StatusApproved, StatusReviewed, nil) + if finalErr.Error() != "Inquiry status is already approved and cannot be changed" { + t.Fatalf("unexpected final status message: %s", finalErr.Error()) + } +}