diff --git a/internal/handler/quiz.go b/internal/handler/quiz.go index da42e28..3db78ef 100644 --- a/internal/handler/quiz.go +++ b/internal/handler/quiz.go @@ -26,7 +26,7 @@ func writeQuizError(w http.ResponseWriter, err error) { case errors.Is(err, repository.ErrQuizNotStarted), errors.Is(err, repository.ErrQuizNotFinished): response.Error(w, http.StatusConflict, err.Error()) case errors.Is(err, repository.ErrQuizNotCurrent), errors.Is(err, repository.ErrQuizAlreadyAnswer), - errors.Is(err, service.ErrQuizLocked): + errors.Is(err, service.ErrQuizLocked), errors.Is(err, repository.ErrQuizTargetTaken): response.Error(w, http.StatusConflict, err.Error()) case errors.Is(err, repository.ErrQuizBadOption), errors.Is(err, repository.ErrQuizChoiceCount), errors.Is(err, service.ErrQuizNoCorrect), diff --git a/internal/repository/quiz_repo.go b/internal/repository/quiz_repo.go index 9a940ef..374deff 100644 --- a/internal/repository/quiz_repo.go +++ b/internal/repository/quiz_repo.go @@ -12,11 +12,14 @@ import ( ) var ( - ErrQuizNotFound = errors.New("quiz not found") - ErrQuizNotStarted = errors.New("start the quiz first") - ErrQuizNotCurrent = errors.New("that isn't the current question") - ErrQuizBadOption = errors.New("that option isn't part of this question") - ErrQuizChoiceCount = errors.New("choose one option, or at least one where several apply") + ErrQuizNotFound = errors.New("quiz not found") + ErrQuizNotStarted = errors.New("start the quiz first") + ErrQuizNotCurrent = errors.New("that isn't the current question") + ErrQuizBadOption = errors.New("that option isn't part of this question") + ErrQuizChoiceCount = errors.New("choose one option, or at least one where several apply") + // ErrQuizTargetTaken: that chapter, lecture or conversation already has a + // quiz (enforced by the quizzes_one_per_target index). + ErrQuizTargetTaken = errors.New("only one quiz is allowed per link") ErrQuizAlreadyAnswer = errors.New("that question is already answered") ErrQuizNotFinished = errors.New("the quiz isn't finished yet") ) @@ -148,6 +151,9 @@ func (r *QuizRepo) Create(ctx context.Context, in QuizInput, createdBy string) ( VALUES ($1, $2, $3, $4, $5, $6) RETURNING id`, in.Title, in.Description, in.TargetKind, in.TargetPath, in.TargetTitle, createdBy, ).Scan(&id); err != nil { + if isUniqueViolation(err, "quizzes_one_per_target") { + return "", ErrQuizTargetTaken + } return "", fmt.Errorf("create quiz: %w", err) } if err := insertQuestions(ctx, tx, id, in.Questions); err != nil { @@ -171,6 +177,9 @@ func (r *QuizRepo) Update(ctx context.Context, id string, in QuizInput, replaceQ WHERE id = $1 AND deleted_at IS NULL`, id, in.Title, in.Description, in.TargetKind, in.TargetPath, in.TargetTitle) if err != nil { + if isUniqueViolation(err, "quizzes_one_per_target") { + return ErrQuizTargetTaken + } return fmt.Errorf("update quiz: %w", err) } if tag.RowsAffected() == 0 { @@ -207,6 +216,23 @@ func insertQuestions(ctx context.Context, tx pgx.Tx, quizID string, qs []Questio return nil } +// QuizOnTarget returns the live quiz (published or draft) already attached to +// a content path, other than exceptID — or "" when the path is free. +func (r *QuizRepo) QuizOnTarget(ctx context.Context, path, exceptID string) (id, title string, err error) { + err = r.pg.Pool.QueryRow(ctx, ` + SELECT id, title FROM quizzes + WHERE target_path = $1 AND deleted_at IS NULL + AND ($2 = '' OR id::text <> $2) + LIMIT 1`, path, exceptID).Scan(&id, &title) + if errors.Is(err, pgx.ErrNoRows) { + return "", "", nil + } + if err != nil { + return "", "", fmt.Errorf("quiz on target: %w", err) + } + return id, title, nil +} + func (r *QuizRepo) AttemptCount(ctx context.Context, id string) (int, error) { var n int err := r.pg.Pool.QueryRow(ctx, `SELECT count(*) FROM quiz_attempts WHERE quiz_id = $1`, id).Scan(&n) diff --git a/internal/service/quiz_service.go b/internal/service/quiz_service.go index 655925a..cdfb530 100644 --- a/internal/service/quiz_service.go +++ b/internal/service/quiz_service.go @@ -199,6 +199,9 @@ func (s *QuizService) Create(ctx context.Context, actorID string, d QuizDraft) ( if err != nil { return nil, err } + if err := s.ensureTargetFree(ctx, in, ""); err != nil { + return nil, err + } id, err := s.quizzes.Create(ctx, in, actorID) if err != nil { return nil, err @@ -223,12 +226,34 @@ func (s *QuizService) Update(ctx context.Context, id string, d QuizDraft) (*mode if err != nil { return nil, err } + if err := s.ensureTargetFree(ctx, in, id); err != nil { + return nil, err + } if err := s.quizzes.Update(ctx, id, in, !locked); err != nil { return nil, err } return s.GetAdmin(ctx, id) } +// ensureTargetFree refuses a second quiz on the same chapter, lecture or +// conversation, naming the one already there so the admin can edit it +// instead. Stand-alone quizzes are never limited. The unique index backs +// this up against a race between two saves. +func (s *QuizService) ensureTargetFree(ctx context.Context, in repository.QuizInput, selfID string) error { + if in.TargetPath == nil { + return nil + } + _, title, err := s.quizzes.QuizOnTarget(ctx, *in.TargetPath, selfID) + if err != nil { + return err + } + if title != "" { + return fmt.Errorf("%w — %q is already linked to %s. Edit that quiz, or delete it first", + repository.ErrQuizTargetTaken, title, *in.TargetPath) + } + return nil +} + func (s *QuizService) SetPublished(ctx context.Context, id string, published bool) (*model.Quiz, error) { if published { q, err := s.quizzes.Get(ctx, id, false) diff --git a/internal/service/quiz_service_test.go b/internal/service/quiz_service_test.go index 31667f2..1728e86 100644 --- a/internal/service/quiz_service_test.go +++ b/internal/service/quiz_service_test.go @@ -3,8 +3,11 @@ package service import ( "context" "errors" + "strings" "testing" + "github.com/google/uuid" + "github.com/prabhupadasays/spsays-bff/internal/model" "github.com/prabhupadasays/spsays-bff/internal/repository" ) @@ -318,3 +321,64 @@ func TestNormalizeContentPath(t *testing.T) { } } } + +// A chapter, lecture or conversation carries at most one live quiz. The +// service names the quiz already there; the unique index catches a race; +// deleting a quiz frees its link; stand-alone quizzes are unlimited. +func TestQuiz_OnePerTarget(t *testing.T) { + svc, pg := newQuizSvc(t) + ctx := context.Background() + admin := chatUser(t, pg, "Admin") + repo := repository.NewQuizRepo(pg) + + path := "/books/zz-test/" + uuid.NewString()[:8] + in := repository.QuizInput{ + Title: "First", TargetKind: model.QuizChapter, TargetPath: &path, + Questions: []repository.QuestionInput{q("p", 0, "a", "b")}, + } + first, err := repo.Create(ctx, in, admin) + if err != nil { + t.Fatal(err) + } + cleanupQuiz(t, pg, first) + + // The friendly pre-check names the quiz that holds the link… + err = svc.ensureTargetFree(ctx, in, "") + if !errors.Is(err, repository.ErrQuizTargetTaken) || !strings.Contains(err.Error(), `"First"`) { + t.Errorf("second quiz on a link: %v, want ErrQuizTargetTaken naming \"First\"", err) + } + // …but a quiz re-saving its own link is fine. + if err := svc.ensureTargetFree(ctx, in, first); err != nil { + t.Errorf("re-saving the same quiz: %v", err) + } + // The index refuses it even if the check is skipped (two saves racing). + in.Title = "Second" + if _, err := repo.Create(ctx, in, admin); !errors.Is(err, repository.ErrQuizTargetTaken) { + t.Errorf("create past the index: %v, want ErrQuizTargetTaken", err) + } + // Moving another quiz onto the link is refused too. + other := repository.QuizInput{Title: "Other", TargetKind: model.QuizStandalone, + Questions: []repository.QuestionInput{q("p", 0, "a", "b")}} + otherID, err := repo.Create(ctx, other, admin) + if err != nil { + t.Fatal(err) + } + cleanupQuiz(t, pg, otherID) + other.TargetKind, other.TargetPath = model.QuizChapter, &path + if err := repo.Update(ctx, otherID, other, false); !errors.Is(err, repository.ErrQuizTargetTaken) { + t.Errorf("moving a quiz onto a taken link: %v, want ErrQuizTargetTaken", err) + } + // Stand-alone quizzes are unlimited. + if solo, err := svc.Create(ctx, admin, QuizDraft{Title: "Solo", Questions: []repository.QuestionInput{q("p", 0, "a", "b")}}); err != nil { + t.Errorf("a second stand-alone quiz: %v", err) + } else { + cleanupQuiz(t, pg, solo.ID) + } + // Deleting the first frees the link. + if err := svc.Delete(ctx, first); err != nil { + t.Fatal(err) + } + if err := repo.Update(ctx, otherID, other, false); err != nil { + t.Errorf("taking a freed link: %v", err) + } +} diff --git a/migrations/0060_quiz_one_per_target.down.sql b/migrations/0060_quiz_one_per_target.down.sql new file mode 100644 index 0000000..58abbec --- /dev/null +++ b/migrations/0060_quiz_one_per_target.down.sql @@ -0,0 +1 @@ +DROP INDEX IF EXISTS quizzes_one_per_target; diff --git a/migrations/0060_quiz_one_per_target.up.sql b/migrations/0060_quiz_one_per_target.up.sql new file mode 100644 index 0000000..596b089 --- /dev/null +++ b/migrations/0060_quiz_one_per_target.up.sql @@ -0,0 +1,6 @@ +-- One quiz per piece of content. A chapter, lecture or conversation may carry +-- at most one live quiz; stand-alone quizzes (no path) are unlimited, and a +-- deleted quiz frees its link for a new one. +CREATE UNIQUE INDEX IF NOT EXISTS quizzes_one_per_target + ON quizzes (target_path) + WHERE deleted_at IS NULL AND target_path IS NOT NULL;