Merge pull request #114 from Dudrie/hotfixes

Fix for inconsistent student-team state

client/src/components/forms/StudentForm.tsx

@@ -59,7 +59,7 @@ interface StudentFormState {
 interface Props extends Omit<FormikBaseFormProps<StudentFormState>, CommonlyUsedFormProps> {
   onSubmit: StudentFormSubmitCallback;
   student?: StudentWithFetchedTeam;
-  students: StudentWithFetchedTeam[];
+  otherStudents: StudentWithFetchedTeam[];
   teams: Team[];
   disableTeamDropdown?: boolean;
 }
@@ -134,7 +134,7 @@ function StudentForm({
   onSubmit,
   className,
   student,
-  students,
+  otherStudents,
   disableTeamDropdown,
   ...other
 }: Props): JSX.Element {
@@ -179,7 +179,7 @@ function StudentForm({
                   return undefined;
                 }
 
-                for (const s of students) {
+                for (const s of otherStudents) {
                   if (s.matriculationNo && value === s.matriculationNo) {
                     return `Matrikelnummer wird bereits von ${getNameOfEntity(s)} verwendet.`;
                   }

client/src/hooks/fetching/Student.ts

@@ -153,7 +153,14 @@ export async function fetchTeamsOfStudents(students: Student[]): Promise<Student
   const promises: Promise<StudentWithFetchedTeam>[] = [];
 
   for (const student of students) {
-    promises.push(getTeamOfStudent(student).then(team => ({ ...student, team })));
+    promises.push(
+      getTeamOfStudent(student)
+        .then(team => ({ ...student, team }))
+        .catch(() => {
+          console.log('Could not load team of student.');
+          return { ...student, team: undefined };
+        })
+    );
   }
 
   return Promise.all(promises);

client/src/view/studentmanagement/AllStudentsAdminView.tsx

@@ -169,7 +169,7 @@ function AllStudentsAdminView({ enqueueSnackbar }: PropType): JSX.Element {
       content: (
         <StudentForm
           student={student}
-          students={students}
+          otherStudents={students.filter(s => s.id !== student.id)}
           teams={student.team ? [student.team] : []}
           onSubmit={editStudent(student)}
           onCancelClicked={() => dialog.hide()}

client/src/view/studentmanagement/Studentmanagement.tsx

@@ -201,7 +201,7 @@ function Studentoverview({ match: { params }, enqueueSnackbar }: PropType): JSX.
       content: (
         <StudentForm
           student={student}
-          students={students}
+          otherStudents={students.filter(s => s.id !== student.id)}
           teams={teams}
           onSubmit={editStudent(student)}
           onCancelClicked={() => dialog.hide()}
@@ -237,7 +237,9 @@ function Studentoverview({ match: { params }, enqueueSnackbar }: PropType): JSX.
         <TableWithForm
           title='Neuen Studierenden anlegen'
           placeholder='Keine Studierenden vorhanden.'
-          form={<StudentForm teams={teams} students={students} onSubmit={handleCreateStudent} />}
+          form={
+            <StudentForm teams={teams} otherStudents={students} onSubmit={handleCreateStudent} />
+          }
           items={students}
           createRowFromItem={student => (
             <ExtendableStudentRow

docker-compose.yaml

@@ -11,6 +11,8 @@ services:
     environment:
       MONGO_INITDB_ROOT_USERNAME: root
       MONGO_INITDB_ROOT_PASSWORD: example
+    volumes: 
+      - db_data:/data/db
 
   mongo-express:
     container_name: mongo_express
@@ -40,7 +42,7 @@ services:
       - ./server/config:/tms/server/config
 
 volumes:
-  db-data:
+  db_data:
 
 networks:
   tms_db:

server/src/model/documents/StudentDocument.ts

@@ -1,11 +1,20 @@
-import { instanceMethod, mapProp, plugin, prop, Ref, Typegoose } from '@hasezoey/typegoose';
+import {
+  instanceMethod,
+  InstanceType,
+  mapProp,
+  plugin,
+  prop,
+  Ref,
+  Typegoose,
+} from '@hasezoey/typegoose';
 import { Document, Model, Types } from 'mongoose';
 import { fieldEncryption } from 'mongoose-field-encryption';
 import { PointId, PointMap, PointMapDTO, PointMapEntry } from 'shared/dist/model/Points';
 import { Sheet } from 'shared/dist/model/Sheet';
 import { Student } from 'shared/dist/model/Student';
 import databaseConfig from '../../helpers/database';
 import { getIdOfDocumentRef } from '../../helpers/documentHelpers';
+import Logger from '../../helpers/Logger';
 import teamService from '../../services/team-service/TeamService.class';
 import { CollectionName } from '../CollectionName';
 import { AttendanceDocument, AttendanceSchema } from './AttendanceDocument';
@@ -67,26 +76,38 @@ export class StudentSchema extends Typegoose
   cakeCount!: number;
 
   @instanceMethod
-  async getTeam(): Promise<TeamDocument> {
+  async getTeam(this: InstanceType<StudentSchema>): Promise<TeamDocument | undefined> {
     if (!this.team) {
-      throw new Error('Can not get team because this student does not belong to a team.');
+      return undefined;
     }
 
-    const [team] = await teamService.getDocumentWithId(
-      getIdOfDocumentRef(this.tutorial),
-      getIdOfDocumentRef(this.team)
-    );
+    try {
+      const [team] = await teamService.getDocumentWithId(
+        getIdOfDocumentRef(this.tutorial),
+        getIdOfDocumentRef(this.team)
+      );
+
+      return team;
+    } catch {
+      Logger.error(
+        `[StudentDocument] Team with ID ${this.team.toString()} does not exist in the DB (anymore). It gets removed from the student.`
+      );
 
-    return team;
+      this.team = undefined;
+      await this.save();
+
+      return undefined;
+    }
   }
 
   @instanceMethod
-  async getPoints(): Promise<PointMap> {
-    if (!this.team) {
+  async getPoints(this: InstanceType<StudentSchema>): Promise<PointMap> {
+    const team = await this.getTeam();
+
+    if (!team) {
       return new PointMap(this.points);
     }
 
-    const team = await this.getTeam();
     const points = new PointMap();
     const pointsOfTeam = new PointMap(team.points);
     const ownPoints = new PointMap(this.points);
@@ -103,26 +124,30 @@ export class StudentSchema extends Typegoose
   }
 
   @instanceMethod
-  async getPointEntry(id: PointId): Promise<PointMapEntry | undefined> {
+  async getPointEntry(
+    this: InstanceType<StudentSchema>,
+    id: PointId
+  ): Promise<PointMapEntry | undefined> {
     const ownMap = new PointMap(this.points);
     const entry = ownMap.getPointEntry(id);
 
     if (entry) {
       return entry;
     }
 
-    if (!this.team) {
+    const team = await this.getTeam();
+
+    if (!team) {
       return undefined;
     }
 
-    const team = await this.getTeam();
     const teamEntry = new PointMap(team.points).getPointEntry(id);
 
     return teamEntry;
   }
 
   @instanceMethod
-  async getPointsOfExercise(id: PointId): Promise<number> {
+  async getPointsOfExercise(this: InstanceType<StudentSchema>, id: PointId): Promise<number> {
     const entry = await this.getPointEntry(id);
 
     return entry ? PointMap.getPointsOfEntry(entry) : 0;

server/src/server.ts

@@ -7,13 +7,19 @@ import ConnectMongo, {
 } from 'connect-mongo';
 import express from 'express';
 import session from 'express-session';
+import { Server } from 'http';
 import mongoose from 'mongoose';
 import passport from 'passport';
 import path from 'path';
 import uuid from 'uuid/v4';
 import databaseConfig from './helpers/database';
+import Logger from './helpers/Logger';
 import initPassport from './helpers/passport';
-import { handleError, EndpointNotFoundError, StartUpError } from './model/Errors';
+import { EndpointNotFoundError, handleError, StartUpError } from './model/Errors';
+import excelRouter from './services/excel-service/ExcelService.routes';
+import languageRouter from './services/language-service/LanguageService.routes';
+import mailRouter from './services/mail-service/MailService.routes';
+import pdfRouter from './services/pdf-service/PdfService.routes';
 import scheincriteriaRouter from './services/scheincriteria-service/ScheincriteriaService.routes';
 import scheinexamRouter from './services/scheinexam-service/ScheinexamService.routes';
 import sheetRouter from './services/sheet-service/SheetService.routes';
@@ -22,11 +28,6 @@ import tutorialRouter from './services/tutorial-service/TutorialService.routes';
 import authenticationRouter from './services/user-service/authentication.routes';
 import userService from './services/user-service/UserService.class';
 import userRouter from './services/user-service/UserService.routes';
-import languageRouter from './services/language-service/LanguageService.routes';
-import mailRouter from './services/mail-service/MailService.routes';
-import Logger from './helpers/Logger';
-import pdfRouter from './services/pdf-service/PdfService.routes';
-import excelRouter from './services/excel-service/ExcelService.routes';
 
 const BASE_API_PATH = '/api';
 const app = express();
@@ -155,6 +156,11 @@ function initEndpoints() {
   registerAPIEndpoint(`${BASE_API_PATH}/excel`, excelRouter);
   registerAPIEndpoint(`${BASE_API_PATH}/locales`, languageRouter);
 
+  // FIXME: REMOVE ME!
+  app.use(`${BASE_API_PATH}/superlongrequest`, (req, res) => {
+    setTimeout(() => res.status(204).send(), 10000);
+  });
+
   // If there's a request which starts with the BASE_API_PATH which did not get handled yet, throw a not found error.
   app.use(BASE_API_PATH, req => {
     throw new EndpointNotFoundError(`Endpoint ${req.url}@${req.method} was not found.`);
@@ -200,7 +206,19 @@ async function startServer() {
 
     await initAdmin();
 
-    app.listen(8080, () => Logger.info('Server started on port 8080.'));
+    const server = app.listen(8080, () => Logger.info('Server started on port 8080.'));
+
+    // Mark every active request so the server can be gracefully stopped.
+    server.on('request', (req, res) => {
+      req.socket._isIdle = false;
+
+      res.on('finish', () => {
+        req.socket._isIdle = true;
+      });
+    });
+
+    process.on('SIGTERM', () => gracefullyStopServer(server));
+    process.on('SIGINT', () => gracefullyStopServer(server));
   } catch (err) {
     if (err.message) {
       Logger.error(`Server start failed. Error: ${err.message}`, { error: err });
@@ -212,4 +230,13 @@ async function startServer() {
   }
 }
 
+function gracefullyStopServer(server: Server) {
+  Logger.info('Termination signal received.');
+  Logger.info('Closing HTTP server...');
+  server.close(() => {
+    Logger.info('HTTP server closed.');
+    Logger.info('Waiting for pending requests...');
+  });
+}
+
 startServer();

server/src/services/student-service/StudentService.class.ts

@@ -101,7 +101,7 @@ class StudentService {
   public async deleteStudent(id: string): Promise<Student> {
     const student: StudentDocument = await this.getDocumentWithId(id);
 
-    if (student.team) {
+    if (await student.getTeam()) {
       await teamService.removeStudentAsMemberFromTeam(student, { saveStudent: true });
     }
 
@@ -219,7 +219,6 @@ class StudentService {
       email,
       courseOfStudies,
       tutorial,
-      team,
       attendance,
       presentationPoints,
       scheinExamResults,
@@ -234,6 +233,7 @@ class StudentService {
       }
     }
 
+    const team = await student.getTeam();
     const points: Student['points'] = (await student.getPoints()).toDTO();
 
     return {
@@ -244,7 +244,7 @@ class StudentService {
       email,
       courseOfStudies,
       tutorial: getIdOfDocumentRef(tutorial),
-      team: team ? getIdOfDocumentRef(team) : undefined,
+      team: team ? team.id : undefined,
       attendance: parsedAttendances,
       points,
       presentationPoints: presentationPoints
@@ -338,15 +338,12 @@ class StudentService {
   }
 
   public async movePointsFromTeamToStudent(student: StudentDocument) {
-    if (!student.team) {
+    const team = await student.getTeam();
+
+    if (!team) {
       return;
     }
 
-    const [team] = await teamService.getDocumentWithId(
-      getIdOfDocumentRef(student.tutorial),
-      student.team.toString()
-    );
-
     const pointsOfTeam = new PointMap(team.points);
     const pointsOfStudent = new PointMap(student.points);
 

server/src/services/team-service/TeamService.class.ts

@@ -217,7 +217,7 @@ class TeamService {
       return;
     }
 
-    if (student.team) {
+    if (await student.getTeam()) {
       await this.removeStudentAsMemberFromTeam(student, { saveStudent: false });
     }
 
@@ -235,14 +235,15 @@ class TeamService {
     student: StudentDocument,
     { saveStudent }: { saveStudent?: boolean } = {}
   ) {
-    if (!student.team) {
+    const oldTeam = await student.getTeam();
+
+    if (!oldTeam) {
       return;
     }
 
-    const [oldTeam, tutorial] = await this.getDocumentWithId(
-      getIdOfDocumentRef(student.tutorial),
-      getIdOfDocumentRef(student.team)
-    );
+    const tutorial = isDocument(oldTeam.tutorial)
+      ? oldTeam.tutorial
+      : await tutorialService.getDocumentWithID(oldTeam.tutorial.toString());
 
     await studentService.movePointsFromTeamToStudent(student);