Add inquiry status transition error handling and validation improvements

- Introduced InvalidStatusTransitionError to handle invalid status transitions for inquiries, providing clearer error messages.
- Updated UpdateInquiryStatusForm to make the Reason field optional and added logic to set a default reason if not provided.
- Enhanced form validation tests to cover new status transition error scenarios and validation messages.
- Refactored handler methods to utilize new error handling functions for improved response management.

This update improves the robustness of inquiry status management by ensuring proper error handling and validation, enhancing user experience during status updates.

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" {

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

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)
+		}
+	}
+}

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)
+	var form CreateInquiryForm
-	if err != nil {
+	if err := c.Bind(&form); err != nil {
-		return response.BadRequest(c, "Invalid request format", "")
+		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)
+	var form UpdateInquiryStatusForm
-	if err != nil {
+	if err := c.Bind(&form); err != nil {
-		return response.BadRequest(c, "Invalid request format", "")
+		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)
+	var form SearchInquiriesForm
-	if err != nil {
+	if err := c.Bind(&form); err != nil {
-		return response.BadRequest(c, "Invalid request format", "")
+		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{}{

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)
+}

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(),

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

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
+}

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())
+	}
+}