diff --git a/src/audit/index.ts b/src/audit/index.ts index 21e5a80..d22cbbf 100644 --- a/src/audit/index.ts +++ b/src/audit/index.ts @@ -32,9 +32,10 @@ type AuditEvent = | "course.module.created" | "course.module.updated" | "course.module.deleted" + | "quiz.feedback.submitted" | "announcement.created" | "announcement.updated" - | "announcement.deleted"; + | "announcement.deleted" | "webhook.created" | "webhook.updated" | "webhook.deleted" diff --git a/src/database/migrations/0022_quiz_feedback.sql b/src/database/migrations/0022_quiz_feedback.sql new file mode 100644 index 0000000..06c8c7d --- /dev/null +++ b/src/database/migrations/0022_quiz_feedback.sql @@ -0,0 +1,18 @@ +-- Quiz feedback: users can flag a specific question as unclear, wrong, or +-- other. One feedback submission per user per (quiz, question) — a second +-- submission is rejected rather than silently overwriting the first. +CREATE TABLE IF NOT EXISTS "quiz_feedback" ( + "id" uuid PRIMARY KEY DEFAULT gen_random_uuid(), + "quiz_id" uuid NOT NULL REFERENCES "quizzes"("id") ON DELETE CASCADE, + "question_id" varchar(100) NOT NULL, + "user_id" uuid NOT NULL REFERENCES "users"("id") ON DELETE CASCADE, + "type" varchar(20) NOT NULL, + "comment" text, + "created_at" timestamp with time zone NOT NULL DEFAULT now(), + CONSTRAINT "chk_quiz_feedback_type" CHECK ("type" IN ('unclear', 'wrong', 'other')) +); + +CREATE UNIQUE INDEX IF NOT EXISTS "idx_quiz_feedback_unique" + ON "quiz_feedback" ("quiz_id", "question_id", "user_id"); +CREATE INDEX IF NOT EXISTS "idx_quiz_feedback_quiz_question" + ON "quiz_feedback" ("quiz_id", "question_id"); diff --git a/src/database/schema.ts b/src/database/schema.ts index 073b781..b42c246 100644 --- a/src/database/schema.ts +++ b/src/database/schema.ts @@ -211,6 +211,44 @@ export const quizSubmissions = pgTable( ] ); +// ─── Quiz Feedback ────────────────────────────────────────────────────────── + +export const quizFeedback = pgTable( + "quiz_feedback", + { + id: uuid("id").primaryKey().defaultRandom(), + quizId: uuid("quiz_id") + .notNull() + .references(() => quizzes.id, { onDelete: "cascade" }), + questionId: varchar("question_id", { length: 100 }).notNull(), + userId: uuid("user_id") + .notNull() + .references(() => users.id, { onDelete: "cascade" }), + type: varchar("type", { length: 20 }).notNull(), + comment: text("comment"), + createdAt: timestamp("created_at", { withTimezone: true }) + .notNull() + .defaultNow(), + }, + (table) => [ + // One feedback submission per (quiz, question, user) — a second + // submission is rejected rather than silently overwriting the first. + uniqueIndex("idx_quiz_feedback_unique").on( + table.quizId, + table.questionId, + table.userId + ), + index("idx_quiz_feedback_quiz_question").on( + table.quizId, + table.questionId + ), + check( + "chk_quiz_feedback_type", + sql`type IN ('unclear', 'wrong', 'other')` + ), + ] +); + // ─── Credentials (NFT Certificates) ──────────────────────────────────────── export const credentials = pgTable( diff --git a/src/modules/courses/course.service.ts b/src/modules/courses/course.service.ts index 4d08215..ca8e336 100644 --- a/src/modules/courses/course.service.ts +++ b/src/modules/courses/course.service.ts @@ -1510,8 +1510,6 @@ export class CourseService { ? { ...data, prerequisites: data.prerequisites.filter((id) => id !== courseId) } : data; - const [updated] = await db - ): Promise { const [course] = await db .update(courses) .set(sanitized) diff --git a/src/modules/quizzes/quiz.controller.ts b/src/modules/quizzes/quiz.controller.ts index cc5aec1..86ed01a 100644 --- a/src/modules/quizzes/quiz.controller.ts +++ b/src/modules/quizzes/quiz.controller.ts @@ -7,6 +7,8 @@ import type { SubmitQuizBody, QuizIdParams, QuizStatsQuery, + SubmitQuizFeedbackBody, + QuizFeedbackSummaryQuery, } from "./quiz.types.js"; export class QuizController { @@ -85,6 +87,37 @@ export class QuizController { reply.send({ success: true, data: stats }); } + + /** + * POST /api/v1/quizzes/:id/feedback + * Submit feedback on a specific quiz question. + */ + async submitFeedback( + request: FastifyRequest<{ Params: QuizIdParams; Body: SubmitQuizFeedbackBody }>, + reply: FastifyReply + ): Promise { + const { authUser } = request as AuthenticatedRequest; + const { id } = request.params; + const data = request.body; + const feedback = await quizService.submitFeedback(authUser.id, id, data); + + reply.status(201).send({ success: true, data: feedback }); + } + + /** + * GET /api/v1/quizzes/:id/feedback/summary + * Per-question feedback counts for a quiz (admin only). + */ + async feedbackSummary( + request: FastifyRequest<{ Params: QuizIdParams; Querystring: QuizFeedbackSummaryQuery }>, + reply: FastifyReply + ): Promise { + const { id } = request.params; + const { questionId } = request.query; + const summary = await quizService.getFeedbackSummary(id, questionId); + + reply.send({ success: true, data: summary }); + } } export const quizController = new QuizController(); diff --git a/src/modules/quizzes/quiz.routes.ts b/src/modules/quizzes/quiz.routes.ts index 74a1348..5b09f89 100644 --- a/src/modules/quizzes/quiz.routes.ts +++ b/src/modules/quizzes/quiz.routes.ts @@ -1,6 +1,6 @@ import type { FastifyInstance, FastifySchema } from "fastify"; import { quizController } from "./quiz.controller.js"; -import { authGuard } from "../../middleware/auth.js"; +import { authGuard, adminGuard } from "../../middleware/auth.js"; import { validate } from "../../middleware/validation.js"; import { quizBatchGenerationRateLimit } from "../../middleware/rate-limit.js"; import { config } from "../../config/index.js"; @@ -10,6 +10,8 @@ import { submitQuizSchema, quizIdParamsSchema, quizStatsQuerySchema, + submitQuizFeedbackSchema, + quizFeedbackSummaryQuerySchema, MAX_BATCH_GENERATE_MODULES, } from "./quiz.types.js"; @@ -149,4 +151,50 @@ export async function quizRoutes(app: FastifyInstance): Promise { }, (request, reply) => quizController.retry(request, reply) ); + + app.post<{ Params: { id: string }, Body: import("./quiz.types.js").SubmitQuizFeedbackBody }>( + "/:id/feedback", + { + preHandler: [ + validate({ params: quizIdParamsSchema, body: submitQuizFeedbackSchema }), + ], + schema: { + description: "Submit feedback on a specific quiz question (#331)", + tags: ["quizzes"], + security: [{ bearerAuth: [] }], + params: { type: "object", required: ["id"], properties: { id: { type: "string", format: "uuid" } } }, + body: { + type: "object", + required: ["questionId", "type"], + properties: { + questionId: { type: "string", minLength: 1, maxLength: 100 }, + type: { type: "string", enum: ["unclear", "wrong", "other"] }, + comment: { type: "string", maxLength: 2000 }, + }, + }, + } as FastifySchema, + }, + (request, reply) => quizController.submitFeedback(request, reply) + ); + + app.get<{ Params: { id: string }, Querystring: import("./quiz.types.js").QuizFeedbackSummaryQuery }>( + "/:id/feedback/summary", + { + preHandler: [ + adminGuard, + validate({ params: quizIdParamsSchema, querystring: quizFeedbackSummaryQuerySchema }), + ], + schema: { + description: "Per-question feedback counts for a quiz (admin only) (#331)", + tags: ["quizzes", "admin"], + security: [{ bearerAuth: [] }], + params: { type: "object", required: ["id"], properties: { id: { type: "string", format: "uuid" } } }, + querystring: { + type: "object", + properties: { questionId: { type: "string", minLength: 1, maxLength: 100 } }, + }, + } as FastifySchema, + }, + (request, reply) => quizController.feedbackSummary(request, reply) + ); } diff --git a/src/modules/quizzes/quiz.service.ts b/src/modules/quizzes/quiz.service.ts index e3b2b0a..3d56f5e 100644 --- a/src/modules/quizzes/quiz.service.ts +++ b/src/modules/quizzes/quiz.service.ts @@ -1,7 +1,7 @@ import crypto from "node:crypto"; import { eq, and } from "drizzle-orm"; import { db } from "../../config/database.js"; -import { quizzes, quizSubmissions, enrollments } from "../../database/schema.js"; +import { quizzes, quizSubmissions, quizFeedback, enrollments } from "../../database/schema.js"; import { NotFoundError, ForbiddenError, @@ -22,8 +22,6 @@ import { cacheSet, cacheDel, cacheKey, - cacheGet, - cacheSet, cacheKeyPattern, cacheInvalidatePattern, } from "../../cache/index.js"; @@ -39,6 +37,9 @@ import { type QuizSubmissionResult, type QuizQuestion, type QuizStats, + type SubmitQuizFeedbackBody, + type QuizFeedbackEntry, + type QuizFeedbackSummaryEntry, } from "./quiz.types.js"; const QUIZ_STATS_TTL_SECONDS = 300; @@ -733,6 +734,117 @@ export class QuizService { return stats; } + /** + * Submit feedback on a specific quiz question (#331): "this question is + * unclear", "wrong answer marked as correct", or something else. + * + * One submission per (quiz, question, user) — a second attempt is + * rejected with a ConflictError rather than overwriting the first, so a + * question's feedback count reflects distinct reporters. + */ + async submitFeedback( + userId: string, + quizId: string, + data: SubmitQuizFeedbackBody, + ): Promise { + const quiz = await db.query.quizzes.findFirst({ + where: eq(quizzes.id, quizId), + }); + if (!quiz) { + throw new NotFoundError("Quiz"); + } + + const questions = (quiz.questions ?? []) as StoredQuestion[]; + if (!questions.some((q) => q.id === data.questionId)) { + throw new NotFoundError("Question"); + } + + const existing = await db.query.quizFeedback.findFirst({ + where: and( + eq(quizFeedback.quizId, quizId), + eq(quizFeedback.questionId, data.questionId), + eq(quizFeedback.userId, userId), + ), + }); + if (existing) { + throw new ConflictError("Feedback already submitted for this question"); + } + + try { + const [row] = await db + .insert(quizFeedback) + .values({ + quizId, + questionId: data.questionId, + userId, + type: data.type, + comment: data.comment ?? null, + }) + .returning(); + + await auditLog("quiz.feedback.submitted", { + userId, + courseId: quiz.courseId, + }); + + return row as QuizFeedbackEntry; + } catch (err) { + const code = (err as { code?: string }).code; + // 23505 = unique_violation — a concurrent submission for the same + // (quiz, question, user) beat this one to the pre-check above. + if (code === "23505") { + throw new ConflictError("Feedback already submitted for this question"); + } + throw err; + } + } + + /** + * Per-question feedback counts for a quiz, for admins reviewing which + * questions need work (#331). + */ + async getFeedbackSummary( + quizId: string, + questionId?: string, + ): Promise { + const quiz = await db.query.quizzes.findFirst({ + where: eq(quizzes.id, quizId), + }); + if (!quiz) { + throw new NotFoundError("Quiz"); + } + + const conditions = [eq(quizFeedback.quizId, quizId)]; + if (questionId) { + conditions.push(eq(quizFeedback.questionId, questionId)); + } + + const rows = await db + .select({ + questionId: quizFeedback.questionId, + type: quizFeedback.type, + }) + .from(quizFeedback) + .where(and(...conditions)); + + const byQuestion = new Map(); + for (const row of rows) { + let entry = byQuestion.get(row.questionId); + if (!entry) { + entry = { + questionId: row.questionId, + total: 0, + counts: { unclear: 0, wrong: 0, other: 0 }, + }; + byQuestion.set(row.questionId, entry); + } + entry.total++; + entry.counts[row.type as QuizFeedbackEntry["type"]]++; + } + + return Array.from(byQuestion.values()); + } + private createPlaceholderQuestions( courseId: string, moduleId: string diff --git a/src/modules/quizzes/quiz.types.ts b/src/modules/quizzes/quiz.types.ts index 72578d4..cd1db61 100644 --- a/src/modules/quizzes/quiz.types.ts +++ b/src/modules/quizzes/quiz.types.ts @@ -67,6 +67,18 @@ export const quizStatsQuerySchema = z.object({ courseId: z.string().uuid("Invalid course ID").optional(), }); +export const QUIZ_FEEDBACK_TYPES = ["unclear", "wrong", "other"] as const; + +export const submitQuizFeedbackSchema = z.object({ + questionId: z.string().min(1).max(100), + type: z.enum(QUIZ_FEEDBACK_TYPES), + comment: z.string().max(2000).optional(), +}); + +export const quizFeedbackSummaryQuerySchema = z.object({ + questionId: z.string().min(1).max(100).optional(), +}); + // ─── Types ────────────────────────────────────────────────────────────────── export type GenerateQuizBody = z.infer; @@ -74,6 +86,8 @@ export type GenerateQuizBatchBody = z.infer; export type SubmitQuizBody = z.infer; export type QuizIdParams = z.infer; export type QuizStatsQuery = z.infer; +export type SubmitQuizFeedbackBody = z.infer; +export type QuizFeedbackSummaryQuery = z.infer; export interface QuizQuestion { id: string; @@ -115,3 +129,19 @@ export interface QuizStats { totalSubmissions: number; submissionsPerCourse: Record; } + +export interface QuizFeedbackEntry { + id: string; + questionId: string; + userId: string; + type: (typeof QUIZ_FEEDBACK_TYPES)[number]; + comment: string | null; + createdAt: Date; +} + +/** Per-question feedback counts, for admins reviewing which questions need work. */ +export interface QuizFeedbackSummaryEntry { + questionId: string; + total: number; + counts: Record<(typeof QUIZ_FEEDBACK_TYPES)[number], number>; +} diff --git a/tests/unit/quizzes/quiz-feedback.test.ts b/tests/unit/quizzes/quiz-feedback.test.ts new file mode 100644 index 0000000..160b28d --- /dev/null +++ b/tests/unit/quizzes/quiz-feedback.test.ts @@ -0,0 +1,193 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("../../../src/config/database.js", () => ({ + db: { + select: vi.fn(), + insert: vi.fn(), + query: { + quizzes: { findFirst: vi.fn() }, + quizFeedback: { findFirst: vi.fn() }, + }, + }, +})); + +vi.mock("../../../src/utils/logger.js", () => ({ + logger: { info: vi.fn(), error: vi.fn(), warn: vi.fn() }, +})); + +vi.mock("../../../src/utils/lock.js", () => ({ + withLock: vi.fn(async (_key: string, fn: () => Promise) => fn()), +})); + +vi.mock("../../../src/audit/index.js", () => ({ + auditLog: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("../../../src/cache/index.js", () => ({ + cacheGet: vi.fn().mockResolvedValue(null), + cacheSet: vi.fn().mockResolvedValue(undefined), + cacheDel: vi.fn().mockResolvedValue(undefined), + cacheKeyPattern: (...p: (string | number)[]) => `${p.join(":")}:*`, + cacheInvalidatePattern: vi.fn().mockResolvedValue(undefined), + cacheKey: (...p: (string | number)[]) => p.join(":"), +})); + +import { db } from "../../../src/config/database.js"; +import { auditLog } from "../../../src/audit/index.js"; +import { quizService } from "../../../src/modules/quizzes/quiz.service.js"; +import { NotFoundError, ConflictError } from "../../../src/utils/errors.js"; + +const mockDb = vi.mocked(db, true); + +const QUIZ = { + id: "quiz-1", + courseId: "course-1", + questions: [{ id: "q1" }, { id: "q2" }], +}; + +describe("QuizService.submitFeedback (#331)", () => { + beforeEach(() => vi.clearAllMocks()); + + it("404s for a quiz that doesn't exist", async () => { + (mockDb.query.quizzes.findFirst as any).mockResolvedValue(undefined); + + await expect( + quizService.submitFeedback("u1", "missing-quiz", { + questionId: "q1", + type: "unclear", + }), + ).rejects.toThrow(NotFoundError); + }); + + it("404s for a question that isn't in the quiz", async () => { + (mockDb.query.quizzes.findFirst as any).mockResolvedValue(QUIZ); + + await expect( + quizService.submitFeedback("u1", "quiz-1", { + questionId: "not-a-real-question", + type: "unclear", + }), + ).rejects.toThrow(NotFoundError); + }); + + it("rejects a second submission from the same user for the same question", async () => { + (mockDb.query.quizzes.findFirst as any).mockResolvedValue(QUIZ); + (mockDb.query.quizFeedback.findFirst as any).mockResolvedValue({ + id: "existing", + }); + + await expect( + quizService.submitFeedback("u1", "quiz-1", { + questionId: "q1", + type: "wrong", + }), + ).rejects.toThrow(ConflictError); + + expect(mockDb.insert).not.toHaveBeenCalled(); + }); + + it("stores feedback and audit-logs it", async () => { + (mockDb.query.quizzes.findFirst as any).mockResolvedValue(QUIZ); + (mockDb.query.quizFeedback.findFirst as any).mockResolvedValue(undefined); + + const returning = vi.fn().mockResolvedValue([ + { + id: "fb-1", + quizId: "quiz-1", + questionId: "q1", + userId: "u1", + type: "unclear", + comment: "confusing wording", + createdAt: new Date("2026-01-01"), + }, + ]); + mockDb.insert.mockReturnValue({ + values: vi.fn().mockReturnValue({ returning }), + } as any); + + const result = await quizService.submitFeedback("u1", "quiz-1", { + questionId: "q1", + type: "unclear", + comment: "confusing wording", + }); + + expect(result.id).toBe("fb-1"); + expect(result.type).toBe("unclear"); + expect(auditLog).toHaveBeenCalledWith( + "quiz.feedback.submitted", + expect.objectContaining({ userId: "u1", courseId: "course-1" }), + ); + }); + + it("converts a concurrent-insert unique violation into a ConflictError", async () => { + (mockDb.query.quizzes.findFirst as any).mockResolvedValue(QUIZ); + (mockDb.query.quizFeedback.findFirst as any).mockResolvedValue(undefined); + + const err = Object.assign(new Error("duplicate key"), { code: "23505" }); + mockDb.insert.mockReturnValue({ + values: vi.fn().mockReturnValue({ + returning: vi.fn().mockRejectedValue(err), + }), + } as any); + + await expect( + quizService.submitFeedback("u1", "quiz-1", { + questionId: "q1", + type: "other", + }), + ).rejects.toThrow(ConflictError); + }); +}); + +describe("QuizService.getFeedbackSummary (#331)", () => { + beforeEach(() => vi.clearAllMocks()); + + function makeSelectChain(result: unknown[]) { + const chain: any = {}; + chain.select = vi.fn().mockReturnValue(chain); + chain.from = vi.fn().mockReturnValue(chain); + chain.where = vi.fn().mockResolvedValue(result); + return chain; + } + + it("404s for a quiz that doesn't exist", async () => { + (mockDb.query.quizzes.findFirst as any).mockResolvedValue(undefined); + + await expect(quizService.getFeedbackSummary("missing-quiz")).rejects.toThrow( + NotFoundError, + ); + }); + + it("groups feedback counts per question and type", async () => { + (mockDb.query.quizzes.findFirst as any).mockResolvedValue(QUIZ); + mockDb.select.mockReturnValue( + makeSelectChain([ + { questionId: "q1", type: "unclear" }, + { questionId: "q1", type: "unclear" }, + { questionId: "q1", type: "wrong" }, + { questionId: "q2", type: "other" }, + ]), + ); + + const summary = await quizService.getFeedbackSummary("quiz-1"); + + expect(summary).toHaveLength(2); + const q1 = summary.find((s) => s.questionId === "q1"); + expect(q1).toEqual({ + questionId: "q1", + total: 3, + counts: { unclear: 2, wrong: 1, other: 0 }, + }); + const q2 = summary.find((s) => s.questionId === "q2"); + expect(q2?.total).toBe(1); + }); + + it("returns an empty summary when there's no feedback yet", async () => { + (mockDb.query.quizzes.findFirst as any).mockResolvedValue(QUIZ); + mockDb.select.mockReturnValue(makeSelectChain([])); + + const summary = await quizService.getFeedbackSummary("quiz-1"); + + expect(summary).toEqual([]); + }); +});