From 357fd1a271f93424f1c9fa174140aabd2038c658 Mon Sep 17 00:00:00 2001 From: "C. Fuhrman" Date: Sun, 26 Jan 2025 21:50:57 -0500 Subject: [PATCH] =?UTF-8?q?Renommer=20dossier=20=C3=A0=20nom=20existant=20?= =?UTF-8?q?donne=20erreur=20Le=20middleware=20des=20erreurs=20ne=20fonctio?= =?UTF-8?q?nnait=20pas=20correctement,=20parce=20que=20les=20routeurs=20ne?= =?UTF-8?q?=20passaient=20pas=20=C3=A0=20next=20J'ai=20ajout=C3=A9=20async?= =?UTF-8?q?Handler=20pour=20passer=20les=20erreurs=20dans=20les=20routeurs?= =?UTF-8?q?=20Supprimer=20messages=20de=20console=20Fix=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/pages/Teacher/Dashboard/Dashboard.tsx | 17 ++++++++--- client/src/services/ApiService.tsx | 7 +++-- server/__tests__/folders.test.js | 28 ++++++++++++++++--- server/constants/errorCodes.js | 16 +++++------ server/controllers/folders.js | 11 ++++++-- server/middleware/AppError.js | 3 +- server/middleware/errorHandler.js | 3 +- server/models/folders.js | 24 ++++++++-------- server/models/quiz.js | 4 +-- server/models/utils.js | 6 ++-- server/routers/folders.js | 16 +++++------ server/routers/images.js | 5 ++-- server/routers/quiz.js | 21 +++++++------- server/routers/routerUtils.js | 6 ++++ server/routers/users.js | 11 ++++---- 15 files changed, 113 insertions(+), 65 deletions(-) create mode 100644 server/routers/routerUtils.js diff --git a/client/src/pages/Teacher/Dashboard/Dashboard.tsx b/client/src/pages/Teacher/Dashboard/Dashboard.tsx index 0300da7..f920d12 100644 --- a/client/src/pages/Teacher/Dashboard/Dashboard.tsx +++ b/client/src/pages/Teacher/Dashboard/Dashboard.tsx @@ -101,12 +101,12 @@ const Dashboard: React.FC = () => { if (selectedFolderId == '') { const folders = await ApiService.getUserFolders(); // HACK force user folders to load on first load - console.log("show all quizes") + //console.log("show all quizzes") let quizzes: QuizType[] = []; for (const folder of folders as FolderType[]) { const folderQuizzes = await ApiService.getFolderContent(folder._id); - console.log("folder: ", folder.title, " quiz: ", folderQuizzes); + //console.log("folder: ", folder.title, " quiz: ", folderQuizzes); // add the folder.title to the QuizType if the folderQuizzes is an array addFolderTitleToQuizzes(folderQuizzes, folder.title); quizzes = quizzes.concat(folderQuizzes as QuizType[]) @@ -294,16 +294,25 @@ const Dashboard: React.FC = () => { try { // folderId: string GET THIS FROM CURRENT FOLDER // currentTitle: string GET THIS FROM CURRENT FOLDER - const newTitle = prompt('Entrée le nouveau nom du fichier', "Nouveau nom de dossier"); + const newTitle = prompt('Entrée le nouveau nom du fichier', folders.find((folder) => folder._id === selectedFolderId)?.title); if (newTitle) { - await ApiService.renameFolder(selectedFolderId, newTitle); + const renamedFolderId = selectedFolderId; + const result = await ApiService.renameFolder(selectedFolderId, newTitle); + + if (result !== true ) { + window.alert(`Une erreur est survenue: ${result}`); + return; + } + const userFolders = await ApiService.getUserFolders(); setFolders(userFolders as FolderType[]); // refresh the page setSelectedFolderId(''); + setSelectedFolderId(renamedFolderId); } } catch (error) { console.error('Error renaming folder:', error); + alert('Erreur lors du renommage du dossier: ' + error); } }; diff --git a/client/src/services/ApiService.tsx b/client/src/services/ApiService.tsx index 4893c32..ef124b4 100644 --- a/client/src/services/ApiService.tsx +++ b/client/src/services/ApiService.tsx @@ -419,7 +419,7 @@ class ApiService { */ public async renameFolder(folderId: string, newTitle: string): Promise { try { - + console.log(`rename folder: folderId: ${folderId}, newTitle: ${newTitle}`); if (!folderId || !newTitle) { throw new Error(`Le folderId et le nouveau titre sont requis.`); } @@ -428,7 +428,9 @@ class ApiService { const headers = this.constructRequestHeaders(); const body = { folderId, newTitle }; - const result: AxiosResponse = await axios.put(url, body, { headers: headers }); + const result = await axios.put(url, body, { headers: headers }); + + console.log(`rename folder: result: ${result.status}, ${result.data}`); if (result.status !== 200) { throw new Error(`Le changement de nom de dossier a échoué. Status: ${result.status}`); } @@ -440,6 +442,7 @@ class ApiService { if (axios.isAxiosError(error)) { const err = error as AxiosError; + console.log(JSON.stringify(err)); const data = err.response?.data as { error: string } | undefined; return data?.error || 'Erreur serveur inconnue lors de la requête.'; } diff --git a/server/__tests__/folders.test.js b/server/__tests__/folders.test.js index 84a7c7f..acde762 100644 --- a/server/__tests__/folders.test.js +++ b/server/__tests__/folders.test.js @@ -197,32 +197,52 @@ describe('Folders', () => { it('should rename a folder and return true', async () => { const folderId = '60c72b2f9b1d8b3a4c8e4d3b'; const newTitle = 'New Folder Name'; + const userId = '12345'; // Mock the database response collection.updateOne.mockResolvedValue({ modifiedCount: 1 }); - const result = await folders.rename(folderId, newTitle); + const result = await folders.rename(folderId, userId, newTitle); expect(db.connect).toHaveBeenCalled(); expect(db.collection).toHaveBeenCalledWith('folders'); - expect(collection.updateOne).toHaveBeenCalledWith({ _id: new ObjectId(folderId) }, { $set: { title: newTitle } }); + // { _id: ObjectId.createFromHexString(folderId), userId: userId }, { $set: { title: newTitle } } + expect(collection.updateOne).toHaveBeenCalledWith({ _id: new ObjectId(folderId), userId: userId }, { $set: { title: newTitle } }); expect(result).toBe(true); }); it('should return false if the folder does not exist', async () => { const folderId = '60c72b2f9b1d8b3a4c8e4d3b'; const newTitle = 'New Folder Name'; + const userId = '12345'; // Mock the database response collection.updateOne.mockResolvedValue({ modifiedCount: 0 }); - const result = await folders.rename(folderId, newTitle); + const result = await folders.rename(folderId, userId, newTitle); expect(db.connect).toHaveBeenCalled(); expect(db.collection).toHaveBeenCalledWith('folders'); - expect(collection.updateOne).toHaveBeenCalledWith({ _id: new ObjectId(folderId) }, { $set: { title: newTitle } }); + expect(collection.updateOne).toHaveBeenCalledWith({ _id: new ObjectId(folderId), userId: userId }, { $set: { title: newTitle } }); expect(result).toBe(false); }); + + it('should throw an error if the new title is already in use', async () => { + const folderId = '60c72b2f9b1d8b3a4c8e4d3b'; + const newTitle = 'Existing Folder'; + const userId = '12345'; + + // Mock the database response + collection.findOne.mockResolvedValue({ title: newTitle }); + collection.updateOne.mockResolvedValue({ modifiedCount: 0 }); + + await expect(folders.rename(folderId, userId, newTitle)).rejects.toThrow(`Folder with name '${newTitle}' already exists.`); + + expect(db.connect).toHaveBeenCalled(); + expect(db.collection).toHaveBeenCalledWith('folders'); + // expect(collection.updateOne).toHaveBeenCalledWith({ _id: new ObjectId(folderId) }, { $set: { title: newTitle } }); + expect(collection.findOne).toHaveBeenCalledWith({ userId: userId, title: newTitle }); + }); }); // duplicate diff --git a/server/constants/errorCodes.js b/server/constants/errorCodes.js index fb691f8..41147ae 100644 --- a/server/constants/errorCodes.js +++ b/server/constants/errorCodes.js @@ -29,7 +29,7 @@ exports.UPDATE_PASSWORD_ERROR = { code: 400 } exports.DELETE_USER_ERROR = { - message: 'Une erreur s\'est produite lors de supression de l\'utilisateur.', + message: 'Une erreur s\'est produite lors de suppression de l\'utilisateur.', code: 400 } @@ -43,15 +43,15 @@ exports.QUIZ_NOT_FOUND = { code: 404 } exports.QUIZ_ALREADY_EXISTS = { - message: 'Le quiz existe déja.', + message: 'Le quiz existe déjà.', code: 400 } exports.UPDATE_QUIZ_ERROR = { - message: 'Une erreur s\'est produite lors de la mise à jours du quiz.', + message: 'Une erreur s\'est produite lors de la mise à jour du quiz.', code: 400 } exports.DELETE_QUIZ_ERROR = { - message: 'Une erreur s\'est produite lors de la supression du quiz.', + message: 'Une erreur s\'est produite lors de la suppression du quiz.', code: 400 } exports.GETTING_QUIZ_ERROR = { @@ -76,15 +76,15 @@ exports.FOLDER_NOT_FOUND = { code: 404 } exports.FOLDER_ALREADY_EXISTS = { - message: 'Le dossier existe déja.', - code: 400 + message: 'Le dossier existe déjà.', + code: 409 } exports.UPDATE_FOLDER_ERROR = { - message: 'Une erreur s\'est produite lors de la mise à jours du dossier.', + message: 'Une erreur s\'est produite lors de la mise à jour du dossier.', code: 400 } exports.DELETE_FOLDER_ERROR = { - message: 'Une erreur s\'est produite lors de la supression du dossier.', + message: 'Une erreur s\'est produite lors de la suppression du dossier.', code: 400 } exports.GETTING_FOLDER_ERROR = { diff --git a/server/controllers/folders.js b/server/controllers/folders.js index 9858821..87528b3 100644 --- a/server/controllers/folders.js +++ b/server/controllers/folders.js @@ -126,8 +126,15 @@ class FoldersController { if (owner != req.user.userId) { throw new AppError(FOLDER_NOT_FOUND); } - - const result = await this.folders.rename(folderId, newTitle); + + // Is this the new title already taken by another folder that I own? + const exists = await this.folders.folderExists(newTitle, req.user.userId); + + if (exists) { + throw new AppError(FOLDER_ALREADY_EXISTS); + } + + const result = await this.folders.rename(folderId, req.user.userId, newTitle); if (!result) { throw new AppError(UPDATE_FOLDER_ERROR); diff --git a/server/middleware/AppError.js b/server/middleware/AppError.js index 9744646..58a4d83 100644 --- a/server/middleware/AppError.js +++ b/server/middleware/AppError.js @@ -2,7 +2,8 @@ class AppError extends Error { constructor (errorCode) { super(errorCode.message) this.statusCode = errorCode.code; + this.isOperational = true; // Optional: to distinguish operational errors from programming errors } } -module.exports = AppError; \ No newline at end of file +module.exports = AppError; diff --git a/server/middleware/errorHandler.js b/server/middleware/errorHandler.js index 61fede6..73c3add 100644 --- a/server/middleware/errorHandler.js +++ b/server/middleware/errorHandler.js @@ -1,8 +1,7 @@ const AppError = require("./AppError"); const fs = require('fs'); -const errorHandler = (error, req, res) => { - console.log("ERROR", error); +const errorHandler = (error, req, res, _next) => { if (error instanceof AppError) { logError(error); diff --git a/server/models/folders.js b/server/models/folders.js index 5ddc225..7a5fb30 100644 --- a/server/models/folders.js +++ b/server/models/folders.js @@ -10,8 +10,6 @@ class Folders { async create(title, userId) { - console.log("LOG: create", title, userId); - if (!title || !userId) { throw new Error('Missing required parameter(s)'); } @@ -86,13 +84,18 @@ class Folders { return true; } - async rename(folderId, newTitle) { + async rename(folderId, userId, newTitle) { await this.db.connect() const conn = this.db.getConnection(); const foldersCollection = conn.collection('folders'); - const result = await foldersCollection.updateOne({ _id: ObjectId.createFromHexString(folderId) }, { $set: { title: newTitle } }) + // see if a folder exists for this user with the new title + const existingFolder = await foldersCollection.findOne({ title: newTitle, userId: userId }); + + if (existingFolder) throw new Error(`Folder with name '${newTitle}' already exists.`); + + const result = await foldersCollection.updateOne({ _id: ObjectId.createFromHexString(folderId), userId: userId }, { $set: { title: newTitle } }) if (result.modifiedCount != 1) return false; @@ -100,7 +103,6 @@ class Folders { } async duplicate(folderId, userId) { - console.log("LOG: duplicate", folderId, userId); const conn = this.db.getConnection(); const foldersCollection = conn.collection('folders'); @@ -112,7 +114,7 @@ class Folders { const theUserId = userId; // Use the utility function to generate a unique title const newFolderTitle = await generateUniqueTitle(sourceFolder.title, async (title) => { - console.log(`generateUniqueTitle(${title}): userId`, theUserId); + // console.log(`generateUniqueTitle(${title}): userId`, theUserId); return await foldersCollection.findOne({ title: title, userId: theUserId }); }); @@ -124,9 +126,9 @@ class Folders { // copy the quizzes from source folder to destination folder const content = await this.getContent(folderId); - console.log("folders.duplicate: found content", content); + // console.log("folders.duplicate: found content", content); for (const quiz of content) { - console.log("folders.duplicate: creating quiz (copy)", quiz); + // console.log("folders.duplicate: creating quiz (copy)", quiz); const result = await this.quizModel.create(quiz.title, quiz.content, newFolderId.toString(), userId); if (!result) { throw new Error('Failed to create duplicate quiz'); @@ -137,14 +139,12 @@ class Folders { } async folderExists(title, userId) { - console.log("LOG: folderExists", title, userId); await this.db.connect(); const conn = this.db.getConnection(); const foldersCollection = conn.collection('folders'); - const existingFolder = await foldersCollection.findOne({ title: title, userId: userId }); - - return !!existingFolder; + const existingFolder = await foldersCollection.findOne({ title: title, userId: userId }); + return existingFolder ? true : false; } diff --git a/server/models/quiz.js b/server/models/quiz.js index 5aabd59..b388659 100644 --- a/server/models/quiz.js +++ b/server/models/quiz.js @@ -9,7 +9,7 @@ class Quiz { } async create(title, content, folderId, userId) { - console.log(`quizzes: create title: ${title}, folderId: ${folderId}, userId: ${userId}`); + // console.log(`quizzes: create title: ${title}, folderId: ${folderId}, userId: ${userId}`); await this.db.connect() const conn = this.db.getConnection(); @@ -31,7 +31,7 @@ class Quiz { } const result = await quizCollection.insertOne(newQuiz); - console.log("quizzes: create insertOne result", result); + // console.log("quizzes: create insertOne result", result); return result.insertedId; } diff --git a/server/models/utils.js b/server/models/utils.js index 8e99a85..219564b 100644 --- a/server/models/utils.js +++ b/server/models/utils.js @@ -1,6 +1,6 @@ // utils.js async function generateUniqueTitle(baseTitle, existsCallback) { - console.log(`generateUniqueTitle(${baseTitle})`); + // console.log(`generateUniqueTitle(${baseTitle})`); let newTitle = baseTitle; let counter = 1; @@ -19,12 +19,12 @@ async function generateUniqueTitle(baseTitle, existsCallback) { newTitle = `${baseTitle} (${counter})`; } - console.log(`first check of newTitle: ${newTitle}`); + // console.log(`first check of newTitle: ${newTitle}`); while (await existsCallback(newTitle)) { counter++; newTitle = `${baseTitle} (${counter})`; - console.log(`trying newTitle: ${newTitle}`); + // console.log(`trying newTitle: ${newTitle}`); } return newTitle; diff --git a/server/routers/folders.js b/server/routers/folders.js index a7898ed..008f0d7 100644 --- a/server/routers/folders.js +++ b/server/routers/folders.js @@ -2,17 +2,17 @@ const express = require('express'); const router = express.Router(); const jwt = require('../middleware/jwtToken.js'); const folders = require('../app.js').folders; +const asyncHandler = require('./routerUtils.js'); -router.post("/create", jwt.authenticate, folders.create); -router.get("/getUserFolders", jwt.authenticate, folders.getUserFolders); -router.get("/getFolderContent/:folderId", jwt.authenticate, folders.getFolderContent); -router.delete("/delete/:folderId", jwt.authenticate, folders.delete); -router.put("/rename", jwt.authenticate, folders.rename); +router.post("/create", jwt.authenticate, asyncHandler(folders.create)); +router.get("/getUserFolders", jwt.authenticate, asyncHandler(folders.getUserFolders)); +router.get("/getFolderContent/:folderId", jwt.authenticate, asyncHandler(folders.getFolderContent)); +router.delete("/delete/:folderId", jwt.authenticate, asyncHandler(folders.delete)); +router.put("/rename", jwt.authenticate, asyncHandler(folders.rename)); -//router.post("/duplicate", jwt.authenticate, foldersController.duplicate); -router.post("/duplicate", jwt.authenticate, folders.duplicate); +router.post("/duplicate", jwt.authenticate, asyncHandler(folders.duplicate)); -router.post("/copy/:folderId", jwt.authenticate, folders.copy); +router.post("/copy/:folderId", jwt.authenticate, asyncHandler(folders.copy)); module.exports = router; diff --git a/server/routers/images.js b/server/routers/images.js index 3723f45..06e2830 100644 --- a/server/routers/images.js +++ b/server/routers/images.js @@ -1,6 +1,7 @@ const express = require('express'); const router = express.Router(); const images = require('../app.js').images; +const asyncHandler = require('./routerUtils.js'); const jwt = require('../middleware/jwtToken.js'); @@ -9,7 +10,7 @@ const multer = require('multer'); const storage = multer.memoryStorage(); const upload = multer({ storage: storage }); -router.post("/upload", jwt.authenticate, upload.single('image'), images.upload); -router.get("/get/:id", images.get); +router.post("/upload", jwt.authenticate, upload.single('image'), asyncHandler(images.upload)); +router.get("/get/:id", asyncHandler(images.get)); module.exports = router; diff --git a/server/routers/quiz.js b/server/routers/quiz.js index 136e4c9..0139c7b 100644 --- a/server/routers/quiz.js +++ b/server/routers/quiz.js @@ -2,21 +2,22 @@ const express = require('express'); const router = express.Router(); const quizzes = require('../app.js').quizzes; const jwt = require('../middleware/jwtToken.js'); +const asyncHandler = require('./routerUtils.js'); if (!quizzes) { console.error("quizzes is not defined"); } -router.post("/create", jwt.authenticate, quizzes.create); -router.get("/get/:quizId", jwt.authenticate, quizzes.get); -router.delete("/delete/:quizId", jwt.authenticate, quizzes.delete); -router.put("/update", jwt.authenticate, quizzes.update); -router.put("/move", jwt.authenticate, quizzes.move); +router.post("/create", jwt.authenticate, asyncHandler(quizzes.create)); +router.get("/get/:quizId", jwt.authenticate, asyncHandler(asyncHandler(quizzes.get))); +router.delete("/delete/:quizId", jwt.authenticate, asyncHandler(quizzes.delete)); +router.put("/update", jwt.authenticate, asyncHandler(quizzes.update)); +router.put("/move", jwt.authenticate, asyncHandler(quizzes.move)); -router.post("/duplicate", jwt.authenticate, quizzes.duplicate); -router.post("/copy/:quizId", jwt.authenticate, quizzes.copy); -router.put("/Share", jwt.authenticate, quizzes.share); -router.get("/getShare/:quizId", jwt.authenticate, quizzes.getShare); -router.post("/receiveShare", jwt.authenticate, quizzes.receiveShare); +router.post("/duplicate", jwt.authenticate, asyncHandler(quizzes.duplicate)); +router.post("/copy/:quizId", jwt.authenticate, asyncHandler(quizzes.copy)); +router.put("/Share", jwt.authenticate, asyncHandler(quizzes.share)); +router.get("/getShare/:quizId", jwt.authenticate, asyncHandler(quizzes.getShare)); +router.post("/receiveShare", jwt.authenticate, asyncHandler(quizzes.receiveShare)); module.exports = router; diff --git a/server/routers/routerUtils.js b/server/routers/routerUtils.js new file mode 100644 index 0000000..335d1c7 --- /dev/null +++ b/server/routers/routerUtils.js @@ -0,0 +1,6 @@ +// asyncHandler is a wrapper for async functions that catch errors and pass them to the next middleware +const asyncHandler = fn => (req, res, next) => { + Promise.resolve(fn(req, res, next)).catch(next); +}; + +module.exports = asyncHandler; diff --git a/server/routers/users.js b/server/routers/users.js index 608daa5..d1f81b7 100644 --- a/server/routers/users.js +++ b/server/routers/users.js @@ -2,11 +2,12 @@ const express = require('express'); const router = express.Router(); const users = require('../app.js').users; const jwt = require('../middleware/jwtToken.js'); +const asyncHandler = require('./routerUtils.js'); -router.post("/register", users.register); -router.post("/login", users.login); -router.post("/reset-password", users.resetPassword); -router.post("/change-password", jwt.authenticate, users.changePassword); -router.post("/delete-user", jwt.authenticate, users.delete); +router.post("/register", asyncHandler(users.register)); +router.post("/login", asyncHandler(users.login)); +router.post("/reset-password", asyncHandler(users.resetPassword)); +router.post("/change-password", jwt.authenticate, asyncHandler(users.changePassword)); +router.post("/delete-user", jwt.authenticate, asyncHandler(users.delete)); module.exports = router;