diff --git a/.eslintrc.json b/.eslintrc.json index 8cbd765..b042fa1 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -11,7 +11,8 @@ "parserOptions": { "ecmaVersion": "latest", "parser": "@typescript-eslint/parser", - "sourceType": "module" + "sourceType": "module", + "project": "./tsconfig.json" }, "plugins": [ "vue", @@ -43,6 +44,7 @@ "semi": [ "error", "always" - ] + ], + "@typescript-eslint/no-floating-promises": "error" } } diff --git a/src/database.ts b/src/database.ts index 3b70262..6a05045 100644 --- a/src/database.ts +++ b/src/database.ts @@ -1,4 +1,4 @@ -import { BaseError, Model, Op, QueryTypes, Sequelize, UniqueConstraintError, WhereOptions } from "sequelize"; +import { BaseError, Model, Op, QueryTypes, Sequelize, Transaction, UniqueConstraintError, WhereOptions } from "sequelize"; import dotenv from "dotenv"; import * as S from "@effect/schema/Schema"; @@ -151,10 +151,11 @@ export async function verifyStudent(verificationCode: string): Promise null); + return update !== null ? VerificationResult.Ok : VerificationResult.Error; } return VerificationResult.InvalidCode; } @@ -170,10 +171,11 @@ export async function verifyEducator(verificationCode: string): Promise null); + return update !== null ? VerificationResult.Ok : VerificationResult.Error; } return VerificationResult.InvalidCode; } @@ -253,33 +255,48 @@ export async function signUpStudent(options: SignUpStudentOptions): Promise { - result = signupResultFromError(error); - }); + const db = Student.sequelize; + if (db === undefined) { + return SignUpResult.Error; + } - // If the student has a valid classroom code, - // add them to the class - if (student && options.classroom_code) { - const cls = await findClassByCode(options.classroom_code); - if (cls !== null) { - StudentsClasses.create({ - student_id: student.id, - class_id: cls.id + try { + const transactionResult = db.transaction(async transaction => { + + const student = await Student.create({ + username: options.username, + verified: 0, + verification_code: verificationCode, + password: encryptedPassword, + institution: options.institution, + email: options.email, + age: options.age, + gender: options.gender, + }, { transaction }) + .catch(error => { + result = signupResultFromError(error); }); - } - } - return result; + // If the student has a valid classroom code, + // add them to the class + if (student && options.classroom_code) { + const cls = await findClassByCode(options.classroom_code); + if (cls !== null) { + await StudentsClasses.create({ + student_id: student.id, + class_id: cls.id + }, { transaction }); + } + } + + return result; + }); + + return transactionResult; + } catch (error) { + console.log(error); + return SignUpResult.Error; + } } export const CreateClassSchema = S.struct({ @@ -294,23 +311,35 @@ export async function createClass(options: CreateClassOptions): Promise { - result = createClassResultFromError(error); - }); - const info = result === CreateClassResult.Ok ? creationInfo : undefined; + const db = Class.sequelize; + if (db === undefined) { + return { result: CreateClassResult.Error }; + } + + try { + const transactionResult = await db.transaction(async transaction => { - // For the pilot, the Hubble Data Story will be the only option, - // so we'll automatically associate that with the class - if (cls) { - ClassStories.create({ - story_name: "hubbles_law", - class_id: cls.id + const cls = await Class.create(creationInfo, { transaction }); + + // For the pilot, the Hubble Data Story will be the only option, + // so we'll automatically associate that with the class + if (cls) { + await ClassStories.create({ + story_name: "hubbles_law", + class_id: cls.id + }, { transaction }); + } + + return creationInfo; }); - } - return { result: result, class: info }; + return { result: result, class: transactionResult }; + } catch (error) { + result = (error instanceof BaseError) ? createClassResultFromError(error) : CreateClassResult.Error; + console.log(error); + return { result: CreateClassResult.Error }; + } } export async function addStudentToClass(studentID: number, classID: number): Promise { @@ -339,7 +368,10 @@ async function checkLogin(identifier: string, password: last_visit: Date.now() }, { where: { id: user.id } - }); + }) + // TODO: We don't want to fail the login if we have an error updating the visit count and time + // But should we do anything else? + .catch(_error => null); } let type: LoginResponse["type"] = "none"; @@ -419,7 +451,10 @@ export async function updateStoryState(studentID: number, storyName: string, new const storyData = { ...query, story_state: newState }; if (result !== null) { - result?.update(storyData); + result?.update(storyData).catch(error => { + console.log(error); + return null; + }); } else { result = await StoryState.create(storyData).catch(error => { console.log(error); @@ -499,7 +534,11 @@ export async function updateStageState(studentID: number, storyName: string, sta const data = { ...query, state: newState }; if (result !== null) { - result?.update(data); + result.update(data) + .catch(error => { + console.log(error); + // TODO: Anything to do here? + }); } else { result = await StageState.create(data).catch(error => { console.log(error); @@ -625,85 +664,116 @@ export async function getClassRoster(classID: number): Promise { } /** These functions are for testing purposes only */ -export async function newDummyClassForStory(storyName: string): Promise<{cls: Class, dummy: DummyClass}> { +export async function newDummyClassForStory(storyName: string, transaction?: Transaction): Promise<{cls: Class, dummy: DummyClass}> { + const trans = transaction ?? null; const ct = await Class.count({ where: { educator_id: 0, name: { [Op.like]: `DummyClass_${storyName}_` } - } + }, + transaction: trans, }); const cls = await Class.create({ educator_id: 0, name: `DummyClass_${storyName}_${ct+1}`, code: "xxxxxx" + }, { transaction: trans }); + let dc = await DummyClass.findOne({ + where: { story_name: storyName }, + transaction: trans, }); - let dc = await DummyClass.findOne({ where: { story_name: storyName }} ); if (dc !== null) { - dc.update({ class_id: cls.id }); + dc.update({ class_id: cls.id }) + .catch(error => { + console.log(error); + // TODO: Anything to do here? + }); } else { dc = await DummyClass.create({ class_id: cls.id, story_name: storyName - }); + }, { transaction: trans }); } return { cls: cls, dummy: dc }; } export async function newDummyStudent(seed = false, teamMember: string | null = null, - storyName: string | null = null): Promise { + storyName: string | null = null): Promise { const students = await Student.findAll(); const ids: number[] = students.map(student => { if (!student) { return 0; } return typeof student.id === "number" ? student.id : 0; }); const newID = Math.max(...ids) + 1; - const student = await Student.create({ - username: `dummy_student_${newID}`, - verified: 1, - verification_code: `verification_${newID}`, - password: "dummypass", - institution: "Dummy", - email: `dummy_student_${newID}@dummy.school`, - age: null, - gender: null, - seed: seed ? 1 : 0, - team_member: teamMember, - dummy: true - }); - - // If we have a story name, and are creating a seed student, we want to add this student to the current "dummy class" for that story - if (seed && storyName !== null) { - let cls: Class | null = null; - let dummyClass = await DummyClass.findOne({ where: { story_name: storyName } }); - let clsSize: number; - if (dummyClass === null) { - const res = await newDummyClassForStory(storyName); - dummyClass = res.dummy; - cls = res.cls; - clsSize = 0; - } else { - clsSize = await StudentsClasses.count({ where: { class_id: dummyClass.class_id } }); - } - - const ct = Math.floor(Math.random() * 11) + 20; - if (clsSize > ct) { - const res = await newDummyClassForStory(storyName); - cls = res.cls; - } else { - cls = await Class.findOne({ where: { id: dummyClass.class_id } }); - } - if (cls !== null) { - StudentsClasses.create({ - class_id: cls.id, - student_id: student.id - }); - } + + const db = Student.sequelize; + if (db === undefined) { + return null; } - return student; + try { + const transactionResult = await db.transaction(async transaction => { + const student = await Student.create({ + username: `dummy_student_${newID}`, + verified: 1, + verification_code: `verification_${newID}`, + password: "dummypass", + institution: "Dummy", + email: `dummy_student_${newID}@dummy.school`, + age: null, + gender: null, + seed: seed ? 1 : 0, + team_member: teamMember, + dummy: true + }, { transaction }); + + // If we have a story name, and are creating a seed student, we want to add this student to the current "dummy class" for that story + if (seed && storyName !== null) { + let cls: Class | null = null; + let dummyClass = await DummyClass.findOne({ + where: { story_name: storyName }, + transaction + }); + let clsSize: number; + if (dummyClass === null) { + const res = await newDummyClassForStory(storyName, transaction); + dummyClass = res.dummy; + cls = res.cls; + clsSize = 0; + } else { + clsSize = await StudentsClasses.count({ + where: { class_id: dummyClass.class_id }, + transaction, + }); + } + + const ct = Math.floor(Math.random() * 11) + 20; + if (clsSize > ct) { + const res = await newDummyClassForStory(storyName); + cls = res.cls; + } else { + cls = await Class.findOne({ + where: { id: dummyClass.class_id }, + transaction, + }); + } + if (cls !== null) { + await StudentsClasses.create({ + class_id: cls.id, + student_id: student.id + }, { transaction }); + } + } + return student; + }); + return transactionResult; + } catch (error) { + console.log(error); + return null; + } } export async function classForStudentStory(studentID: number, storyName: string): Promise { @@ -752,7 +822,11 @@ export async function setStudentOption(studentID: number, option: StudentOption, options = await createStudentOptions(studentID); } if (options !== null) { - options.update({ [option]: value }); + options.update({ [option]: value }) + .catch(error => { + console.log(error); + // TODO: Anything to do here? + }); } return options; } diff --git a/src/main.ts b/src/main.ts index 486c5ce..6dabb03 100644 --- a/src/main.ts +++ b/src/main.ts @@ -17,6 +17,12 @@ promises.readdir(STORIES_DIR, { withFileTypes: true }).then(entries => { app.use(data.path, data.router); } }); +}) + +// We should just fail if this step doesn't succeed +.catch(error => { + console.error(error); + throw new Error("Error setting up sub-routers!"); }); // set port, listen for requests diff --git a/src/request_results.ts b/src/request_results.ts index 85f6f88..715ea85 100644 --- a/src/request_results.ts +++ b/src/request_results.ts @@ -83,7 +83,8 @@ export enum VerificationResult { BadRequest = "bad_request", Ok = "ok", AlreadyVerified = "already_verified", - InvalidCode = "invalid_code" + InvalidCode = "invalid_code", + Error = "internal_server_error", } export namespace VerificationResult { diff --git a/src/server.ts b/src/server.ts index ceac367..fa8c278 100644 --- a/src/server.ts +++ b/src/server.ts @@ -273,6 +273,8 @@ export function createApp(db: Sequelize): Express { return 401; case VerificationResult.AlreadyVerified: return 409; + case VerificationResult.Error: + return 500; } } @@ -458,18 +460,33 @@ export function createApp(db: Sequelize): Express { }); app.delete("/classes/:code", async (req, res) => { - const cls = await findClassByCode(req.params.code); - const success = cls !== null; + const code = req.params.code; + const cls = await findClassByCode(code); + if (cls === null) { + res.status(404).json({ + success: false, + error: `Could not find class with code ${code}`, + }); + return; + } + const success = await cls.destroy() + .then(() => true) + .catch(error => { + console.log(error); + return false; + }); + if (!success) { - res.status(400); + res.status(500).json({ + success: false, + error: `Server error deleting class with code ${code}`, + }); + return; } - cls?.destroy(); - const message = success ? - "Class deleted" : - "No class with the given code exists"; + res.json({ - success, - message + success: true, + message: "Class deleted", }); }); diff --git a/src/stories/hubbles_law/database.ts b/src/stories/hubbles_law/database.ts index 33d20eb..f149d2a 100644 --- a/src/stories/hubbles_law/database.ts +++ b/src/stories/hubbles_law/database.ts @@ -623,16 +623,31 @@ export async function getSampleGalaxy(): Promise { }); } -export async function markGalaxyBad(galaxy: Galaxy): Promise { - galaxy.update({ marked_bad: galaxy.marked_bad + 1 }); +export async function markGalaxyBad(galaxy: Galaxy): Promise { + return galaxy.update({ marked_bad: galaxy.marked_bad + 1 }) + .then(() => true) + .catch(error => { + console.log(error); + return false; + }); } -export async function markGalaxySpectrumBad(galaxy: Galaxy): Promise { - galaxy.update({ spec_marked_bad: galaxy.spec_marked_bad + 1 }); +export async function markGalaxySpectrumBad(galaxy: Galaxy): Promise { + return galaxy.update({ spec_marked_bad: galaxy.spec_marked_bad + 1 }) + .then(() => true) + .catch(error => { + console.log(error); + return false; + }); } -export async function markGalaxyTileloadBad(galaxy: Galaxy): Promise { - galaxy.update({ tileload_marked_bad: galaxy.tileload_marked_bad + 1 }); +export async function markGalaxyTileloadBad(galaxy: Galaxy): Promise { + return galaxy.update({ tileload_marked_bad: galaxy.tileload_marked_bad + 1 }) + .then(() => true) + .catch(error => { + console.log(error); + return false; + }); } export async function getAsyncMergedClassIDForStudent(studentID: number): Promise { diff --git a/src/stories/hubbles_law/router.ts b/src/stories/hubbles_law/router.ts index b9f1650..108b20c 100644 --- a/src/stories/hubbles_law/router.ts +++ b/src/stories/hubbles_law/router.ts @@ -451,7 +451,7 @@ router.get("/galaxies", async (req, res) => { res.json(galaxies); }); -async function markBad(req: GenericRequest, res: GenericResponse, marker: (galaxy: Galaxy) => Promise, markedStatus: string) { +async function markBad(req: GenericRequest, res: GenericResponse, marker: (galaxy: Galaxy) => Promise) { const galaxyID = req.body.galaxy_id; const galaxyName = req.body.galaxy_name; if (!(galaxyID || galaxyName)) { @@ -475,10 +475,7 @@ async function markBad(req: GenericRequest, res: GenericResponse, marker: (galax return; } - marker(galaxy); - res.status(200).json({ - status: markedStatus - }); + return marker(galaxy); } /** @@ -486,11 +483,29 @@ async function markBad(req: GenericRequest, res: GenericResponse, marker: (galax * This was previously idempotent, but no longer is */ router.put("/mark-galaxy-bad", async (req, res) => { - markBad(req, res, markGalaxyBad, "galaxy_marked_bad"); + const success = await markBad(req, res, markGalaxyBad); + + if (success) { + res.status(204).end(); + } else { + res.status(500).json({ + error: "Error marking galaxy as bad", + }); + } + }); router.post("/mark-spectrum-bad", async (req, res) => { - markBad(req, res, markGalaxySpectrumBad, "galaxy_spectrum_marked_bad"); + const success = await markBad(req, res, markGalaxySpectrumBad); + + if (success) { + res.status(204).end(); + } else { + res.status(500).json({ + error: "Error marking spectrum as bad", + }); + } + }); router.get("/spectra/:type/:name", async (req, res) => { @@ -506,7 +521,16 @@ router.get("/unchecked-galaxies", async (_req, res) => { }); router.post("/mark-tileload-bad", async (req, res) => { - markBad(req, res, markGalaxyTileloadBad, "galaxy_tileload_marked_bad"); + const success = await markBad(req, res, markGalaxyTileloadBad); + + if (success) { + res.status(204).end(); + } else { + res.status(500).json({ + error: "Error marking spectrum as bad", + }); + } + }); router.post("/set-spectrum-status", async (req, res) => { @@ -533,7 +557,20 @@ router.post("/set-spectrum-status", async (req, res) => { return; } - setGalaxySpectrumStatus(galaxy, good); + const success = await setGalaxySpectrumStatus(galaxy, good) + .then(() => true) + .catch(error => { + console.log(error); + return false; + }); + + if (!success) { + res.status(500).json({ + error: `Error setting galaxy spectrum status for ${name} to ${good ? "" : " not "}good`, + }); + return; + } + res.json({ status: "status_updated", marked_good: good, diff --git a/src/tests/root.test.ts b/src/tests/root.test.ts index 61e4e23..3dc6148 100644 --- a/src/tests/root.test.ts +++ b/src/tests/root.test.ts @@ -4,7 +4,7 @@ import testApp, { authorizedRequest } from "./utils"; describe("Test root route", async() => { it("Should show a welcome message", async () => { - authorizedRequest(testApp) + void authorizedRequest(testApp) .get("/") .expect(200) .expect("Content-Type", /json/) diff --git a/src/tests/utils.ts b/src/tests/utils.ts index 6a1e00d..0444f1a 100644 --- a/src/tests/utils.ts +++ b/src/tests/utils.ts @@ -19,7 +19,11 @@ export function unauthorizedRequest(app: Express) { function createTestDatabase(): Sequelize { const db = new Sequelize({ dialect: "mysql" }); - db.query("CREATE DATABASE IF NOT EXISTS test"); + db.query("CREATE DATABASE IF NOT EXISTS test") + .catch(error => { + console.log(error); + throw new Error(`Error creating test database: ${error}`); + }); return db; }