Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/exam admission semantic checks #188

Merged

Conversation

NikoBergemann
Copy link
Collaborator

Reason for this PR

  • missing semantic checks for examAdmissions

Changes in this PR

  • Check, if enrollmentId is matriculated in an examinationRegulation containing the moduleId, referenced in the exam behind the examId.

  • Check, if enrollmentId is not admitted to the same exam already.

  • Check, if enrollmentId is admitted to the couseId, referenced in the exam behind the examId.

  • Check, if the examId is currently admittable

  • update tests

Co-authored-by: Anemone Kampkötter <anemone@kampkoetter.de>
@NikoBergemann NikoBergemann changed the base branch from develop to feature/exam_prerelease February 17, 2021 14:33
@NikoBergemann NikoBergemann self-assigned this Feb 17, 2021
Comment on lines +185 to +188
List<ExamAdmission> examAdmissions = this.getAllStates(stub, ExamAdmission.class);
return examAdmissions.stream().noneMatch(item ->
item.getExamId().equals(admission.getExamId())
&& item.getEnrollmentId().equals(admission.getEnrollmentId()));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could just build the admission key and query for that directly, but this should of course also work.

Comment on lines +198 to +201
List<CourseAdmission> courseAdmissions = this.getAllStates(stub, CourseAdmission.class);
return courseAdmissions.stream().anyMatch(item ->
item.getCourseId().equals(exam.getCourseId())
&& item.getEnrollmentId().equals(admission.getEnrollmentId()));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing here

Comment on lines +96 to +113
if (!cUtil.checkExamExists(stub, this)) {
invalidParameters.add(cUtil.getAdmissionExamNotExistsParam());
}
if (!cUtil.checkExamAvailableForStudent(stub, this)) {
invalidParameters.add(cUtil.getAdmissionExamNotAvailableParam("enrollmentId"));
invalidParameters.add(cUtil.getAdmissionExamNotAvailableParam("examId"));
}
if (!cUtil.checkExamAdmissionNotAlreadyExists(stub, this)) {
invalidParameters.add(cUtil.getAdmissionAlreadyExistsParam("enrollmentId"));
invalidParameters.add(cUtil.getAdmissionAlreadyExistsParam("examId"));
}
if (!cUtil.checkCourseAdmissionExists(stub, this)) {
invalidParameters.add(cUtil.getCourseAdmissionNotExistsParam("enrollmentId"));
invalidParameters.add(cUtil.getCourseAdmissionNotExistsParam("examId"));
}
if (!cUtil.checkExamAdmittable(stub, this)) {
invalidParameters.add(cUtil.getAdmissionNotPossibleParam());
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens for e.g. empty examId? Will we get both the error that it should not be empty, and every semantic error related to not finding the exam?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I am not satisfied with that - imho we should only submit the exact error (only empty) but that increases the complexity. I guess we can deal with this kind of "better error management" later on, once all the functionality is established

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +329 to +331
{
"name":"admission.examId",
"reason":"The exam you are trying to admit for does not exist."
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guess this solves my last question....

@NikoBergemann NikoBergemann removed the request for review from anemonekk February 19, 2021 15:23
@NikoBergemann NikoBergemann merged commit a83e4e0 into feature/exam_prerelease Feb 19, 2021
@NikoBergemann NikoBergemann deleted the feature/examAdmission_semanticChecks branch February 19, 2021 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ExamAdmission - garbage enrollmentId ExamAdmissions - garbage examId ExamAdmission - Timestamp
2 participants