Admin-managed feedback categories

Categories move from a hard-coded list to a feedback_categories table (migration 0062), seeded with the four existing keys, labels and descriptions so production feedback is untouched; any other key already in use gets an archived category. Keys are fixed, so renames relabel past feedback. Archive hides a category from the form; only unused categories can be deleted. Submissions are validated against active categories, and feedback responses carry category_label. New: GET /feedback/categories (public); /admin/feedback/categories CRUD + reorder behind the new feedback:categories permission (granted to Admin). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-18 13:47:09 +05:30
parent fa0255e9cd
commit d76505772a
13 changed files with 610 additions and 40 deletions
@@ -0,0 +1,124 @@
+package service
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"regexp"
+	"strings"
+
+	"github.com/prabhupadasays/spsays-bff/internal/model"
+	"github.com/prabhupadasays/spsays-bff/internal/repository"
+)
+
+const (
+	maxCategoryLabel       = 60
+	maxCategoryDescription = 200
+)
+
+var (
+	ErrCategoryLabel       = fmt.Errorf("a category needs a name (up to %d characters)", maxCategoryLabel)
+	ErrCategoryDescription = fmt.Errorf("the description can be up to %d characters", maxCategoryDescription)
+	ErrCategoryLastActive  = errors.New("the feedback form needs at least one category")
+)
+
+var categoryNonSlug = regexp.MustCompile(`[^a-z0-9]+`)
+
+// FeedbackCategoryService manages the /feedback form's categories.
+type FeedbackCategoryService struct {
+	repo *repository.FeedbackCategoryRepo
+}
+
+func NewFeedbackCategoryService(repo *repository.FeedbackCategoryRepo) *FeedbackCategoryService {
+	return &FeedbackCategoryService{repo: repo}
+}
+
+// Active is what the /feedback form offers, in order.
+func (s *FeedbackCategoryService) Active(ctx context.Context) ([]model.FeedbackCategory, error) {
+	return s.repo.List(ctx, false)
+}
+
+// All is the admin's view: archived ones too, with usage counts.
+func (s *FeedbackCategoryService) All(ctx context.Context) ([]model.FeedbackCategory, error) {
+	return s.repo.List(ctx, true)
+}
+
+func cleanCategory(label, description string) (string, string, error) {
+	label = strings.TrimSpace(label)
+	description = strings.TrimSpace(description)
+	if label == "" || len([]rune(label)) > maxCategoryLabel {
+		return "", "", ErrCategoryLabel
+	}
+	if len([]rune(description)) > maxCategoryDescription {
+		return "", "", ErrCategoryDescription
+	}
+	return label, description, nil
+}
+
+// Create adds a category to the end of the form. Its key is derived from
+// the name ("Audio problem" → audio_problem) and fixed from then on, so it
+// can be renamed freely later without touching past feedback.
+func (s *FeedbackCategoryService) Create(ctx context.Context, label, description string) (*model.FeedbackCategory, error) {
+	label, description, err := cleanCategory(label, description)
+	if err != nil {
+		return nil, err
+	}
+	base := strings.Trim(categoryNonSlug.ReplaceAllString(strings.ToLower(label), "_"), "_")
+	if base == "" {
+		base = "category"
+	}
+	if len(base) > 40 {
+		base = strings.TrimRight(base[:40], "_")
+	}
+	key := base
+	for i := 2; ; i++ {
+		taken, err := s.repo.KeyTaken(ctx, key)
+		if err != nil {
+			return nil, err
+		}
+		if !taken {
+			break
+		}
+		key = fmt.Sprintf("%s_%d", base, i)
+	}
+	return s.repo.Create(ctx, key, label, description)
+}
+
+// Update renames, rewords, archives or restores a category. The last active
+// category can't be archived — the form would have nothing to offer.
+func (s *FeedbackCategoryService) Update(ctx context.Context, key, label, description string, archived bool) error {
+	label, description, err := cleanCategory(label, description)
+	if err != nil {
+		return err
+	}
+	if archived {
+		if err := s.guardLastActive(ctx, key); err != nil {
+			return err
+		}
+	}
+	return s.repo.Update(ctx, key, label, description, archived)
+}
+
+// Delete removes a category nothing is filed under (otherwise: archive it).
+func (s *FeedbackCategoryService) Delete(ctx context.Context, key string) error {
+	if err := s.guardLastActive(ctx, key); err != nil {
+		return err
+	}
+	return s.repo.Delete(ctx, key)
+}
+
+// Reorder sets the form's order.
+func (s *FeedbackCategoryService) Reorder(ctx context.Context, keys []string) error {
+	return s.repo.Reorder(ctx, keys)
+}
+
+func (s *FeedbackCategoryService) guardLastActive(ctx context.Context, key string) error {
+	active, err := s.repo.List(ctx, false)
+	if err != nil {
+		return err
+	}
+	if len(active) == 1 && active[0].Key == key {
+		return ErrCategoryLastActive
+	}
+	return nil
+}
@@ -0,0 +1,106 @@
+package service
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/prabhupadasays/spsays-bff/internal/repository"
+	"go.uber.org/zap"
+)
+
+// Categories are managed from the admin: a new one is offered on the form
+// and accepts feedback; renaming relabels past feedback; archiving stops new
+// submissions but keeps the label; in-use categories can't be deleted.
+func TestFeedbackCategories(t *testing.T) {
+	_, _, pg := testChatService(t)
+	ctx := context.Background()
+	repo := repository.NewFeedbackCategoryRepo(pg)
+	cats := NewFeedbackCategoryService(repo)
+	fb := NewFeedbackService(repository.NewFeedbackRepo(pg), repo, nil, nil, zap.NewNop())
+	user := chatUser(t, pg, "Feedback")
+	t.Cleanup(func() {
+		bg := context.Background()
+		_, _ = pg.Pool.Exec(bg, `DELETE FROM feedback WHERE user_id = $1`, user)
+		_, _ = pg.Pool.Exec(bg, `DELETE FROM feedback_categories WHERE key LIKE 'zz_qa_audio%'`)
+	})
+
+	// The seeded categories production already uses are all on the form.
+	active, err := cats.Active(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	seen := map[string]bool{}
+	for _, c := range active {
+		seen[c.Key] = true
+	}
+	for _, k := range []string{"general", "bug", "feature", "other"} {
+		if !seen[k] {
+			t.Errorf("seeded category %q missing from the form", k)
+		}
+	}
+
+	// Keys come from the name, and never collide.
+	c1, err := cats.Create(ctx, "ZZ QA Audio problem", "Playback trouble")
+	if err != nil {
+		t.Fatal(err)
+	}
+	c2, err := cats.Create(ctx, "ZZ QA audio problem!", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if c1.Key != "zz_qa_audio_problem" || c2.Key != "zz_qa_audio_problem_2" {
+		t.Errorf("keys = %q, %q", c1.Key, c2.Key)
+	}
+	if _, err := cats.Create(ctx, "   ", ""); !errors.Is(err, ErrCategoryLabel) {
+		t.Errorf("blank name: %v, want ErrCategoryLabel", err)
+	}
+
+	// New categories take feedback; unknown ones don't.
+	if _, err := fb.Submit(ctx, &user, c1.Key, "the audio stutters", nil); err != nil {
+		t.Fatalf("submit in a new category: %v", err)
+	}
+	if _, err := fb.Submit(ctx, &user, "no_such_category", "x", nil); !errors.Is(err, ErrFeedbackCategory) {
+		t.Errorf("unknown category: %v, want ErrFeedbackCategory", err)
+	}
+
+	// Renaming relabels what's already been filed.
+	if err := cats.Update(ctx, c1.Key, "ZZ QA Audio & playback", "Playback trouble", false); err != nil {
+		t.Fatal(err)
+	}
+	mine, _ := fb.ListForUser(ctx, user, 10)
+	if len(mine) != 1 || mine[0].CategoryLabel != "ZZ QA Audio & playback" || mine[0].Category != c1.Key {
+		t.Errorf("past feedback after rename = %+v", mine)
+	}
+
+	// Archived: off the form, refuses new feedback, keeps its label.
+	if err := cats.Update(ctx, c1.Key, "ZZ QA Audio & playback", "", true); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := fb.Submit(ctx, &user, c1.Key, "again", nil); !errors.Is(err, ErrFeedbackCategory) {
+		t.Errorf("submit to an archived category: %v, want ErrFeedbackCategory", err)
+	}
+	active, _ = cats.Active(ctx)
+	for _, c := range active {
+		if c.Key == c1.Key {
+			t.Error("archived category still on the form")
+		}
+	}
+	if mine, _ := fb.ListForUser(ctx, user, 10); mine[0].CategoryLabel != "ZZ QA Audio & playback" {
+		t.Error("archiving lost the label on past feedback")
+	}
+	all, _ := cats.All(ctx)
+	for _, c := range all {
+		if c.Key == c1.Key && (c.ArchivedAt == nil || c.UsageCount != 1) {
+			t.Errorf("admin view of archived category = %+v", c)
+		}
+	}
+
+	// In use → can't be deleted; unused → can.
+	if err := cats.Delete(ctx, c1.Key); !errors.Is(err, repository.ErrFeedbackCategoryInUse) {
+		t.Errorf("deleting an in-use category: %v, want ErrFeedbackCategoryInUse", err)
+	}
+	if err := cats.Delete(ctx, c2.Key); err != nil {
+		t.Errorf("deleting an unused category: %v", err)
+	}
+}
@@ -21,24 +21,23 @@ import (
 )

 type FeedbackService struct {
-	repo   *repository.FeedbackRepo
-	mail   mailer.Mailer
-	admins []string
-	logger *zap.Logger
+	repo *repository.FeedbackRepo
+	// categories is the admin-managed list a submission must belong to.
+	categories *repository.FeedbackCategoryRepo
+	mail       mailer.Mailer
+	admins     []string
+	logger     *zap.Logger
 }

-func NewFeedbackService(repo *repository.FeedbackRepo, mail mailer.Mailer, admins []string, logger *zap.Logger) *FeedbackService {
-	return &FeedbackService{repo: repo, mail: mail, admins: admins, logger: logger}
+func NewFeedbackService(repo *repository.FeedbackRepo, categories *repository.FeedbackCategoryRepo, mail mailer.Mailer, admins []string, logger *zap.Logger) *FeedbackService {
+	return &FeedbackService{repo: repo, categories: categories, mail: mail, admins: admins, logger: logger}
 }

 // Allowed categories — kept lowercase in storage so dashboards group
 // cleanly. The frontend dropdown sends one of these exact strings.
-var feedbackCategories = map[string]bool{
-	"bug":     true,
-	"feature": true,
-	"general": true,
-	"other":   true,
-}
+// ErrFeedbackCategory: the submission names a category that doesn't exist
+// or has been archived in the admin panel.
+var ErrFeedbackCategory = errors.New("invalid category")

 // Submit persists a feedback row and fires the admin email.
 //
@@ -49,8 +48,10 @@ var feedbackCategories = map[string]bool{
 // trustworthy (the form can't lie about who's writing).
 func (s *FeedbackService) Submit(ctx context.Context, userID *string, category, message string, contactEmail *string) (*model.Feedback, error) {
 	category = strings.ToLower(strings.TrimSpace(category))
-	if !feedbackCategories[category] {
-		return nil, errors.New("invalid category")
+	if ok, err := s.categories.IsActive(ctx, category); err != nil {
+		return nil, err
+	} else if !ok {
+		return nil, ErrFeedbackCategory
 	}
 	message = strings.TrimSpace(message)
 	if message == "" {
@@ -76,16 +77,15 @@ func (s *FeedbackService) Submit(ctx context.Context, userID *string, category,
 		return nil, err
 	}

-	// Hydrate the joined author fields for the email template. Failing
-	// the lookup isn't worth failing the request — we fall back to
-	// "anonymous" rather than 500ing on the user.
-	if userID != nil {
-		if full, err := s.repo.FindByID(ctx, f.ID); err == nil {
-			f = full
-		} else {
-			s.logger.Warn("feedback hydrate failed; sending email without author info",
-				zap.String("feedback_id", f.ID), zap.Error(err))
-		}
+	// Hydrate the joined author fields and category label for the email
+	// template. Failing the lookup isn't worth failing the request — we fall
+	// back to "anonymous" and the category key rather than 500ing.
+	if full, err := s.repo.FindByID(ctx, f.ID); err == nil {
+		f = full
+	} else {
+		s.logger.Warn("feedback hydrate failed; sending email without author info",
+			zap.String("feedback_id", f.ID), zap.Error(err))
+		f.CategoryLabel = f.Category
 	}

 	s.notifyAdmins(f)
@@ -222,7 +222,7 @@ func (s *FeedbackService) notifyAdmins(f *model.Feedback) {
 		s.logger.Warn("render feedback email failed", zap.Error(err))
 		return
 	}
-	subject := fmt.Sprintf("[Feedback · %s] %s", f.Category, summarize(f.Message, 60))
+	subject := fmt.Sprintf("[Feedback · %s] %s", categoryLabel(f), summarize(f.Message, 60))
 	for _, to := range s.admins {
 		if err := s.mail.Send(to, subject, body); err != nil {
 			s.logger.Warn("feedback email send failed",
@@ -263,7 +263,7 @@ func renderFeedbackEmail(f *model.Feedback) (string, error) {
 	}
 	var buf bytes.Buffer
 	err := feedbackEmailTpl.Execute(&buf, map[string]any{
-		"Category":  f.Category,
+		"Category":  categoryLabel(f),
 		"Message":   f.Message,
 		"From":      from,
 		"CreatedAt": f.CreatedAt.Format("2006-01-02 15:04:05 MST"),
@@ -360,3 +360,11 @@ func renderFeedbackResolvedEmail(f *model.Feedback) (string, error) {
 	}
 	return buf.String(), nil
 }
+
+// categoryLabel is the category's display name, or its key if unknown.
+func categoryLabel(f *model.Feedback) string {
+	if f.CategoryLabel != "" {
+		return f.CategoryLabel
+	}
+	return f.Category
+}