Skip to content

Commit

Permalink
Merge pull request #980 from Dudrie/fix-unecessary-fetch-of-students
Browse files Browse the repository at this point in the history
Fix unecessary fetch of students in ScheinResultsPDFGenerator.
  • Loading branch information
Dudrie authored Feb 19, 2022
2 parents 8e98320 + 87195c9 commit 5c8ad2a
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 17 deletions.
15 changes: 6 additions & 9 deletions server/src/module/pdf/subservices/PDFGenerator.schein.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,12 @@ export class ScheinResultsPDFGenerator extends PDFWithStudentsGenerator<Generato
*
* @returns Buffer of a PDF containing the list with the schein status of all the given students.
*/
public async generatePDF({
enableShortMatriculationNo: enableShortMatriculatinNo,
}: GeneratorOptions): Promise<Buffer> {
const [allStudents, summaries] = await Promise.all([
this.studentService.findAll(),
this.scheincriteriaService.getResultsOfAllStudents(),
]);
public async generatePDF({ enableShortMatriculationNo }: GeneratorOptions): Promise<Buffer> {
const summaries = await this.scheincriteriaService.getResultsOfAllStudents();

const students = allStudents.filter((student) => !!student.matriculationNo);
const students = Object.values(summaries)
.map((s) => s.student)
.filter((student) => !!student.matriculationNo);
const shortenedMatriculationNumbers = this.getShortenedMatriculationNumbers(students);
const statuses: Scheinstatus[] = [];
const template = this.templateService.getScheinstatusTemplate();
Expand All @@ -50,7 +47,7 @@ export class ScheinResultsPDFGenerator extends PDFWithStudentsGenerator<Generato
state = summaries[studentId].passed ? PassedState.PASSED : PassedState.NOT_PASSED;
}

if (enableShortMatriculatinNo) {
if (enableShortMatriculationNo) {
statuses.push({ matriculationNo: shortenedNo, state });
} else {
const matriculationNo = students.find((s) => s.id === studentId)?.matriculationNo;
Expand Down
19 changes: 11 additions & 8 deletions server/src/module/pdf/subservices/PDFGenerator.withStudents.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { PDFGenerator } from './PDFGenerator.core';
import { IStudent } from 'shared/model/Student';
import { StudentDocument } from '../../../database/models/student.model';

interface ShortendMatriculationInfo {
Expand All @@ -10,14 +11,14 @@ export abstract class PDFWithStudentsGenerator<T> extends PDFGenerator<T> {
/**
* Returns the shortened number for all students together with the ID of the student to which the shortened matriculation number belongs to.
*
* Those shortened numbers are still enough to identify a student. However, this is only true if one only consideres the given students. If one extends that array without re-running this function the identifying feature may get lost.
* Those shortened numbers are still enough to identify a student. However, this is only true if one only considers the given students. If one extends that array without re-running this function the identifying feature may get lost.
*
* @param students All students to get the shortened number from.
*
* @returns The shortened but still identifying matriculation numbers of all given students.
*/
protected getShortenedMatriculationNumbers(
students: StudentDocument[]
students: (StudentDocument | IStudent)[]
): ShortendMatriculationInfo[] {
const result: ShortendMatriculationInfo[] = [];
const matriculationNos: { id: string; reversedNumber: string }[] = [];
Expand All @@ -26,7 +27,7 @@ export abstract class PDFWithStudentsGenerator<T> extends PDFGenerator<T> {
if (student.matriculationNo) {
matriculationNos.push({
id: student.id,
reversedNumber: this.reverseString(student.matriculationNo),
reversedNumber: PDFWithStudentsGenerator.reverseString(student.matriculationNo),
});
}
}
Expand All @@ -39,22 +40,24 @@ export abstract class PDFWithStudentsGenerator<T> extends PDFGenerator<T> {

if (idx !== 0) {
const prev = matriculationNos[idx - 1];
positionPrev = this.getFirstDifferentPosition(
positionPrev = PDFWithStudentsGenerator.getFirstDifferentPosition(
current.reversedNumber,
prev.reversedNumber
);
}

if (idx !== matriculationNos.length - 1) {
const next = matriculationNos[idx + 1];
positionNext = this.getFirstDifferentPosition(
positionNext = PDFWithStudentsGenerator.getFirstDifferentPosition(
current.reversedNumber,
next.reversedNumber
);
}

const position: number = Math.max(positionPrev, positionNext);
const substring = this.reverseString(current.reversedNumber.substr(0, position + 1));
const substring = PDFWithStudentsGenerator.reverseString(
current.reversedNumber.substr(0, position + 1)
);

result.push({
studentId: current.id,
Expand All @@ -72,7 +75,7 @@ export abstract class PDFWithStudentsGenerator<T> extends PDFGenerator<T> {
* @param second Second string
* @returns The first position in which both string differ. If they are completly equal the length of the first string is returned.
*/
private getFirstDifferentPosition(first: string, second: string): number {
private static getFirstDifferentPosition(first: string, second: string): number {
for (let i = 0; i < first.length; i++) {
if (first.charAt(i) !== second.charAt(i)) {
return i;
Expand All @@ -86,7 +89,7 @@ export abstract class PDFWithStudentsGenerator<T> extends PDFGenerator<T> {
* @param string String to reverse
* @returns The reversed string.
*/
private reverseString(string: string): string {
private static reverseString(string: string): string {
return string.split('').reverse().join('');
}
}

0 comments on commit 5c8ad2a

Please sign in to comment.