From 495c6d2e41ffee578e453a11996807e6e7260641 Mon Sep 17 00:00:00 2001 From: Lauro Gripa Date: Sun, 27 Oct 2024 20:18:35 -0300 Subject: [PATCH] Improve progress endpoint (#53) * Count exp points only for finished lessons * Improve lesson progress endpoint * Return 409 when duplicate * Prevent further submissions when lesson is already complete --- src/controllers/progress.ts | 59 +++++++-- src/routes.ts | 4 +- tests/progress.test.ts | 233 ++++++++++++++++++++++++++++++++++-- 3 files changed, 277 insertions(+), 19 deletions(-) diff --git a/src/controllers/progress.ts b/src/controllers/progress.ts index 5fe4491..49465c9 100644 --- a/src/controllers/progress.ts +++ b/src/controllers/progress.ts @@ -3,6 +3,8 @@ import { ProgressModel } from "@/models/Progress"; import { LessonModel } from "@/models/Lesson"; import { Course, CourseModel } from "@/models/Course"; import { UserModel } from "@/models/User"; +import { Types } from "mongoose"; +import { MongoError } from "mongodb"; export const submitAnswer = async (req: Request, res: Response) => { const { courseId, lessonId, choice, userId } = req.body; @@ -21,6 +23,10 @@ export const submitAnswer = async (req: Request, res: Response) => { return res.status(400).send({ error: { message: "Course not found" } }); } + // prevent the user from submitting again if the lesson is already complete + const progress = await ProgressModel.findOne({ courseId, lessonId, userId, isCorrect: true }); + if (progress) return res.status(200).send(progress); + const isCorrect = choice === lesson.challenge.correctChoice; let errorMessage; @@ -33,8 +39,16 @@ export const submitAnswer = async (req: Request, res: Response) => { isCorrect, difficulty: lesson.difficulty, }); - if (newProgress) return res.status(200).send(newProgress); + if (newProgress) return res.status(201).send(newProgress); } catch (e) { + if (e instanceof MongoError && e.code === 11000) { + return res.status(409).send({ + error: { + message: "E11000 duplicate key error", + }, + }); + } + errorMessage = (e as Error).message; console.error(`[ERROR][submitAnswer] ${JSON.stringify(e)}`); } @@ -47,14 +61,14 @@ export const submitAnswer = async (req: Request, res: Response) => { }; export const getLessonProgress = async (req: Request, res: Response) => { - const { userId, lessonId } = req.params; + const { courseId, lessonId, userId } = req.params; - if (!userId || !lessonId) { + if (!courseId || !lessonId || !userId) { return res.status(400).send({ error: { message: "Missing params" } }); } try { - const progress = await ProgressModel.find({ userId, lessonId }); + const progress = await ProgressModel.find({ courseId, lessonId, userId }); return res.status(200).send(progress); } catch (e) { console.error(`[ERROR][getLessonProgress] ${JSON.stringify(e)}`); @@ -70,7 +84,7 @@ export const getCourseProgress = async (req: Request, res: Response) => { } try { - const progress = await ProgressModel.find({ userId, courseId }); + const progress = await ProgressModel.find({ userId, courseId, isCorrect: true }); const course = (await CourseModel.findOne({ _id: courseId }).populate({ path: "modules", populate: { @@ -130,13 +144,40 @@ export const getUserXPAndLevel = async (req: Request, res: Response) => { return res.status(400).send({ error: { message: "User not found" } }); } - const progress = await ProgressModel.find({ userId }); + const progress = await ProgressModel.aggregate([ + { + $match: { userId: new Types.ObjectId(userId) }, + }, + { + $group: { + _id: { + courseId: "$courseId", + lessonId: "$lessonId", + difficulty: "$difficulty", + }, + count: { $sum: 1 }, + correctCount: { $sum: { $cond: ["$isCorrect", 1, 0] } }, + }, + }, + { + $project: { + courseId: "$_id.courseId", + lessonId: "$_id.lessonId", + difficulty: "$_id.difficulty", + correctAtFirstTry: { $cond: [{ $eq: ["$count", 1] }, { $eq: ["$correctCount", 1] }, false] }, + isCorrect: { $gt: ["$correctCount", 0] }, + _id: 0, + }, + }, + ]); let exp = 0; for (const p of progress) { - const difficulty = p.difficulty as Difficulty; - const points = p.isCorrect ? EXP_POINTS[difficulty].perfect : EXP_POINTS[difficulty].withMistakes; - exp += points; + if (p.isCorrect) { + const difficulty = p.difficulty as Difficulty; + const points = p.correctAtFirstTry ? EXP_POINTS[difficulty].perfect : EXP_POINTS[difficulty].withMistakes; + exp += points; + } } const level = calculateLevel(exp); diff --git a/src/routes.ts b/src/routes.ts index 7597377..1a340cb 100644 --- a/src/routes.ts +++ b/src/routes.ts @@ -61,9 +61,9 @@ const router = (app: Express) => { // Progress app.post("/progress", [authMiddleware], submitAnswer); - app.get("/progress/lesson/:userId/:lessonId", [authMiddleware], getLessonProgress); + app.get("/progress/lesson/:userId/:courseId/:lessonId", [authMiddleware], getLessonProgress); app.get("/progress/course/:userId/:courseId", [authMiddleware], getCourseProgress); - app.get("/progress/xp-level/:userId", [authMiddleware], getUserXPAndLevel); + app.get("/progress/level/:userId", [authMiddleware], getUserXPAndLevel); }; export default router; diff --git a/tests/progress.test.ts b/tests/progress.test.ts index 5cd3702..7a1c891 100644 --- a/tests/progress.test.ts +++ b/tests/progress.test.ts @@ -95,7 +95,7 @@ describe("Setting API Server up...", () => { title: "Lesson #3", language: "english", body: loadFixture("example.md"), - difficulty: "easy", + difficulty: "medium", challenge: { question: "Another question?", choices: ["1", "2", "3"], @@ -107,7 +107,7 @@ describe("Setting API Server up...", () => { title: "Lesson #4", language: "english", body: loadFixture("example.md"), - difficulty: "medium", + difficulty: "hard", challenge: { question: "Another question?", choices: ["1", "2", "3"], @@ -174,15 +174,15 @@ describe("Setting API Server up...", () => { expect(r.data.userId).toEqual(user?.id.toString()); expect(r.data.choice).toEqual(wrongChoice); expect(r.data.isCorrect).toEqual(false); + expect(r.status).toEqual(201); }) .catch((e) => { expect(e).toBeUndefined(); }); }); - it("Unique lesson, course, user, and choice (POST /progress)", async () => { + it("Prevent submitting another answer when lesson is complete (POST /progress)", async () => { const choice = 0; - await axios .post(`${API_URL}/progress`, { courseId: course._id, @@ -196,27 +196,99 @@ describe("Setting API Server up...", () => { expect(r.data.userId).toEqual(user?.id.toString()); expect(r.data.choice).toEqual(choice); expect(r.data.isCorrect).toEqual(true); + expect(r.status).toEqual(201); }) .catch((e) => { expect(e).toBeUndefined(); }); + const wrongChoice = 1; await axios .post(`${API_URL}/progress`, { courseId: course._id, lessonId: lesson1._id, userId: user?.id, - choice: choice, + choice: wrongChoice, + }) + .then((r) => { + expect(r.data.courseId).toEqual(course._id?.toString()); + expect(r.data.lessonId).toEqual(lesson1._id?.toString()); + expect(r.data.userId).toEqual(user?.id.toString()); + expect(r.data.choice).toEqual(choice); + expect(r.data.isCorrect).toEqual(true); + expect(r.status).toEqual(200); + }) + .catch((e) => { + expect(e).toBeUndefined(); + }); + }); + + it("Unique lesson, course, user, and choice (POST /progress)", async () => { + const wrongChoice = 1; + + await axios + .post(`${API_URL}/progress`, { + courseId: course._id, + lessonId: lesson1._id, + userId: user?.id, + choice: wrongChoice, + }) + .then((r) => { + expect(r.data.courseId).toEqual(course._id?.toString()); + expect(r.data.lessonId).toEqual(lesson1._id?.toString()); + expect(r.data.userId).toEqual(user?.id.toString()); + expect(r.data.choice).toEqual(wrongChoice); + expect(r.data.isCorrect).toEqual(false); + }) + .catch((e) => { + expect(e).toBeUndefined(); + }); + + await axios + .post(`${API_URL}/progress`, { + courseId: course._id, + lessonId: lesson1._id, + userId: user?.id, + choice: wrongChoice, }) .then(() => { throw new Error("Duplicate progress entry was allowed, but it should not be."); }) .catch((e) => { - expect(e.response.status).toEqual(400); + expect(e.response.status).toEqual(409); expect(e.response.data.error.message).toContain("E11000 duplicate key error"); }); }); + it("Get lesson progress (GET /progress/lesson/:userId/:courseId/:lessonId)", async () => { + await ProgressModel.create({ + courseId: course._id, + lessonId: lesson1._id, + userId: user?.id, + choice: lesson1.challenge.correctChoice + 1, + isCorrect: false, + difficulty: lesson1.difficulty, + }); + + await ProgressModel.create({ + courseId: course._id, + lessonId: lesson1._id, + userId: user?.id, + choice: lesson1.challenge.correctChoice, + isCorrect: true, + difficulty: lesson1.difficulty, + }); + + await axios + .get(`${API_URL}/progress/lesson/${user?.id}/${course._id}/${lesson1._id}`) + .then((r) => { + expect(r.data.length).toEqual(2); + }) + .catch((e) => { + expect(e).toBeUndefined(); + }); + }); + it("Get course progress with no completed lessons (GET /progress/course/:userId/:courseId)", async () => { await axios .get(`${API_URL}/progress/course/${user?.id}/${course._id}`) @@ -252,6 +324,36 @@ describe("Setting API Server up...", () => { }); }); + it("Get course progress with one lesson incomplete (GET /progress/course/:userId/:courseId)", async () => { + await ProgressModel.create({ + courseId: course._id, + lessonId: lesson1._id, + userId: user?.id, + choice: 0, + isCorrect: true, + difficulty: lesson1.difficulty, + }); + await ProgressModel.create({ + courseId: course._id, + lessonId: lesson2._id, + userId: user?.id, + choice: 2, + isCorrect: false, + difficulty: lesson2.difficulty, + }); + + await axios + .get(`${API_URL}/progress/course/${user?.id}/${course._id}`) + .then((r) => { + expect(r.data.totalLessons).toEqual(5); + expect(r.data.completedLessons).toEqual(1); + expect(r.data.progressPercentage).toEqual(20); + }) + .catch((e) => { + expect(e).toBeUndefined(); + }); + }); + it("Get course progress with all lessons completed (GET /progress/course/:userId/:courseId)", async () => { await ProgressModel.create({ courseId: course._id, @@ -261,6 +363,7 @@ describe("Setting API Server up...", () => { isCorrect: true, difficulty: lesson1.difficulty, }); + await ProgressModel.create({ courseId: course._id, lessonId: lesson2._id, @@ -270,12 +373,126 @@ describe("Setting API Server up...", () => { difficulty: lesson2.difficulty, }); + await ProgressModel.create({ + courseId: course._id, + lessonId: lesson3._id, + userId: user?.id, + choice: 2, + isCorrect: true, + difficulty: lesson3.difficulty, + }); + + await ProgressModel.create({ + courseId: course._id, + lessonId: lesson4._id, + userId: user?.id, + choice: 0, + isCorrect: true, + difficulty: lesson4.difficulty, + }); + + await ProgressModel.create({ + courseId: course._id, + lessonId: lesson5._id, + userId: user?.id, + choice: 1, + isCorrect: true, + difficulty: lesson5.difficulty, + }); + await axios .get(`${API_URL}/progress/course/${user?.id}/${course._id}`) .then((r) => { + expect(r.status).toEqual(200); expect(r.data.totalLessons).toEqual(5); - expect(r.data.completedLessons).toEqual(2); - expect(r.data.progressPercentage).toEqual(40); + expect(r.data.completedLessons).toEqual(5); + expect(r.data.progressPercentage).toEqual(100); + }) + .catch((e) => { + expect(e).toBeUndefined(); + }); + }); + + it("Get user XP and level (GET /progress/level/:userId)", async () => { + // Easy lesson mistake (0 XP) + await ProgressModel.create({ + courseId: course._id, + lessonId: lesson1._id, + userId: user?.id, + choice: lesson1.challenge.correctChoice + 1, + isCorrect: false, + difficulty: lesson1.difficulty, + }); + + // Easy lesson correct (25 XP) + await ProgressModel.create({ + courseId: course._id, + lessonId: lesson1._id, + userId: user?.id, + choice: lesson1.challenge.correctChoice, + isCorrect: true, + difficulty: lesson1.difficulty, + }); + + // Easy lesson correct at first try (50 XP) + await ProgressModel.create({ + courseId: course._id, + lessonId: lesson2._id, + userId: user?.id, + choice: lesson2.challenge.correctChoice, + isCorrect: true, + difficulty: lesson2.difficulty, + }); + + // Medium lesson mistake (0 XP) + await ProgressModel.create({ + courseId: course._id, + lessonId: lesson3._id, + userId: user?.id, + choice: lesson3.challenge.correctChoice + 1, + isCorrect: false, + difficulty: lesson3.difficulty, + }); + + // Medium lesson another mistake (0 XP) + await ProgressModel.create({ + courseId: course._id, + lessonId: lesson3._id, + userId: user?.id, + choice: lesson3.challenge.correctChoice + 2, + isCorrect: false, + difficulty: lesson3.difficulty, + }); + + // Medium lesson correct (50 XP) + await ProgressModel.create({ + courseId: course._id, + lessonId: lesson3._id, + userId: user?.id, + choice: lesson3.challenge.correctChoice, + isCorrect: true, + difficulty: lesson3.difficulty, + }); + + // Hard lesson mistake, incomplete (0 XP) + await ProgressModel.create({ + courseId: course._id, + lessonId: lesson4._id, + userId: user?.id, + choice: lesson4.challenge.correctChoice + 1, + isCorrect: false, + difficulty: lesson4.difficulty, + }); + + // Total: 125 XP + await axios + .get(`${API_URL}/progress/level/${user?.id}`) + .then((r) => { + expect(r.status).toEqual(200); + expect(r.data).toHaveProperty("exp"); + expect(r.data).toHaveProperty("level"); + expect(r.data.exp).toEqual(125); + expect(r.data.level).toEqual(0); }) .catch((e) => { expect(e).toBeUndefined();