diff --git a/src/main/java/de/tum/cit/aet/artemis/exam/service/ExamAccessService.java b/src/main/java/de/tum/cit/aet/artemis/exam/service/ExamAccessService.java index b16e5af39eff..ca97f86bce7c 100644 --- a/src/main/java/de/tum/cit/aet/artemis/exam/service/ExamAccessService.java +++ b/src/main/java/de/tum/cit/aet/artemis/exam/service/ExamAccessService.java @@ -84,17 +84,26 @@ public StudentExam getExamInCourseElseThrow(Long courseId, Long examId) { studentExam = optionalStudentExam.get(); } else { - // Only Test Exams can be self-created by the user. Exam examWithExerciseGroupsAndExercises = examRepository.findWithExerciseGroupsAndExercisesByIdOrElseThrow(examId); + // An exam can be started 5 minutes before the start time, which is when programming exercises are unlocked + boolean canExamBeStarted = ZonedDateTime.now().isAfter(ExamDateService.getExamProgrammingExerciseUnlockDate(examWithExerciseGroupsAndExercises)); + boolean isExamEnded = ZonedDateTime.now().isAfter(examWithExerciseGroupsAndExercises.getEndDate()); + // Generate a student exam if the following conditions are met: + // 1. The exam has not ended. + // 2. The exam is either a test exam, OR it is a normal exam where the user is registered and can click the start button. + // Allowing student exams to be generated only when students can click the start button prevents inconsistencies. + // For example, this avoids a scenario where a student generates an exam and an instructor adds an exercise group afterward. + if (!isExamEnded + && (examWithExerciseGroupsAndExercises.isTestExam() || (examRegistrationService.isUserRegisteredForExam(examId, currentUser.getId()) && canExamBeStarted))) { + studentExam = studentExamService.generateIndividualStudentExam(examWithExerciseGroupsAndExercises, currentUser); + // For the start of the exam, the exercises are not needed. They are later loaded via StudentExamResource + studentExam.setExercises(null); - if (!examWithExerciseGroupsAndExercises.isTestExam()) { + } + else { // We skip the alert since this can happen when a tutor sees the exam card or the user did not participate yet is registered for the exam - throw new BadRequestAlertException("The requested Exam is no test exam and thus no student exam can be created", ENTITY_NAME, - "StudentExamGenerationOnlyForTestExams", true); + throw new BadRequestAlertException("Cannot generate student exam for exam ID " + examId + ".", ENTITY_NAME, "cannotGenerateStudentExam", true); } - studentExam = studentExamService.generateTestExam(examWithExerciseGroupsAndExercises, currentUser); - // For the start of the exam, the exercises are not needed. They are later loaded via StudentExamResource - studentExam.setExercises(null); } Exam exam = studentExam.getExam(); diff --git a/src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRegistrationService.java b/src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRegistrationService.java index 2f747de288ff..18b6aafbbe29 100644 --- a/src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRegistrationService.java +++ b/src/main/java/de/tum/cit/aet/artemis/exam/service/ExamRegistrationService.java @@ -72,9 +72,12 @@ public class ExamRegistrationService { private static final boolean IS_TEST_RUN = false; + private final StudentExamService studentExamService; + public ExamRegistrationService(ExamUserRepository examUserRepository, ExamRepository examRepository, UserService userService, ParticipationService participationService, UserRepository userRepository, AuditEventRepository auditEventRepository, CourseRepository courseRepository, StudentExamRepository studentExamRepository, - StudentParticipationRepository studentParticipationRepository, AuthorizationCheckService authorizationCheckService, ExamUserService examUserService) { + StudentParticipationRepository studentParticipationRepository, AuthorizationCheckService authorizationCheckService, ExamUserService examUserService, + StudentExamService studentExamService) { this.examRepository = examRepository; this.userService = userService; this.userRepository = userRepository; @@ -86,6 +89,7 @@ public ExamRegistrationService(ExamUserRepository examUserRepository, ExamReposi this.authorizationCheckService = authorizationCheckService; this.examUserRepository = examUserRepository; this.examUserService = examUserService; + this.studentExamService = studentExamService; } /** @@ -193,6 +197,7 @@ public boolean isUserRegisteredForExam(Long examId, Long userId) { * Registers student to the exam. In order to do this, we add the user to the course group, because the user only has access to the exam of a course if the student also has * access to the course of the exam. * We only need to add the user to the course group, if the student is not yet part of it, otherwise the student cannot access the exam (within the course). + * If the exam has already started, a student exam is additionally generated. * * @param course the course containing the exam * @param exam the exam for which we want to register a student @@ -216,6 +221,11 @@ public void registerStudentToExam(Course course, Exam exam, User student) { registeredExamUser = examUserRepository.save(registeredExamUser); exam.addExamUser(registeredExamUser); examRepository.save(exam); + // Generate a student exam for the registered student if the exam has already started + if (exam.isStarted()) { + Exam examWithExerciseGroupsAndExercises = examRepository.findWithExerciseGroupsAndExercisesByIdOrElseThrow(exam.getId()); + studentExamService.generateIndividualStudentExam(examWithExerciseGroupsAndExercises, student); + } } else { log.warn("Student {} is already registered for the exam {}", student.getLogin(), exam.getId()); diff --git a/src/main/java/de/tum/cit/aet/artemis/exam/service/StudentExamService.java b/src/main/java/de/tum/cit/aet/artemis/exam/service/StudentExamService.java index 73e0ffaa8d8b..a2898a9df5ff 100644 --- a/src/main/java/de/tum/cit/aet/artemis/exam/service/StudentExamService.java +++ b/src/main/java/de/tum/cit/aet/artemis/exam/service/StudentExamService.java @@ -9,7 +9,6 @@ import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -797,16 +796,17 @@ public void setUpExerciseParticipationsAndSubmissions(StudentExam studentExam, L } /** - * Generates a new test exam for the student and stores it in the database + * Generates a new individual StudentExam for the specified student and stores it in the database. * - * @param exam the exam with loaded exercise groups and exercises for which the StudentExam should be created - * @param student the corresponding student - * @return a StudentExam for the student and exam + * @param exam The exam with eagerly loaded users, exercise groups, and exercises. + * @param student The student for whom the StudentExam should be created. + * @return The generated StudentExam. */ - public StudentExam generateTestExam(Exam exam, User student) { + public StudentExam generateIndividualStudentExam(Exam exam, User student) { // To create a new StudentExam, the Exam with loaded ExerciseGroups and Exercises is needed long start = System.nanoTime(); - StudentExam studentExam = generateIndividualStudentExam(exam, student); + Set userSet = Collections.singleton(student); + StudentExam studentExam = studentExamRepository.createRandomStudentExams(exam, userSet, examQuizQuestionsGenerator).getFirst(); // we need to break a cycle for the serialization studentExam.getExam().setExerciseGroups(null); studentExam.getExam().setStudentExams(null); @@ -816,20 +816,6 @@ public StudentExam generateTestExam(Exam exam, User student) { return studentExam; } - /** - * Generates an individual StudentExam - * - * @param exam with eagerly loaded users, exerciseGroups and exercises loaded - * @param student the student for which the StudentExam should be created - * @return the generated StudentExam - */ - private StudentExam generateIndividualStudentExam(Exam exam, User student) { - // StudentExams are saved in the called method - HashSet userHashSet = new HashSet<>(); - userHashSet.add(student); - return studentExamRepository.createRandomStudentExams(exam, userHashSet, examQuizQuestionsGenerator).getFirst(); - } - /** * Generates the student exams randomly based on the exam configuration and the exercise groups * Important: the passed exams needs to include the registered users, exercise groups and exercises (eagerly loaded) diff --git a/src/test/java/de/tum/cit/aet/artemis/exam/ExamRegistrationIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/exam/ExamRegistrationIntegrationTest.java index 8402a7e431f3..8114e2e942ff 100644 --- a/src/test/java/de/tum/cit/aet/artemis/exam/ExamRegistrationIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/exam/ExamRegistrationIntegrationTest.java @@ -5,6 +5,7 @@ import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.verify; +import java.time.ZonedDateTime; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -30,8 +31,10 @@ import de.tum.cit.aet.artemis.core.user.util.UserFactory; import de.tum.cit.aet.artemis.exam.domain.Exam; import de.tum.cit.aet.artemis.exam.domain.ExamUser; +import de.tum.cit.aet.artemis.exam.domain.StudentExam; import de.tum.cit.aet.artemis.exam.repository.ExamRepository; import de.tum.cit.aet.artemis.exam.repository.ExamUserRepository; +import de.tum.cit.aet.artemis.exam.repository.StudentExamRepository; import de.tum.cit.aet.artemis.exam.service.ExamRegistrationService; import de.tum.cit.aet.artemis.exam.util.ExamFactory; import de.tum.cit.aet.artemis.exam.util.ExamUtilService; @@ -61,6 +64,9 @@ class ExamRegistrationIntegrationTest extends AbstractSpringIntegrationLocalCILo @Autowired private ExamUtilService examUtilService; + @Autowired + private StudentExamRepository studentExamRepository; + private Course course1; private Exam exam1; @@ -108,6 +114,23 @@ void testRegisterUserInExam_addedToCourseStudentsGroup() throws Exception { assertThat(studentsInCourseBefore).containsExactlyInAnyOrderElementsOf(studentsInCourseAfter); } + @Test + @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR") + void testRegisterUserInExam_studentExamGenerated() throws Exception { + Exam exam = examUtilService.addExam(course1); + exam = examUtilService.addTextModelingProgrammingExercisesToExam(exam, false, false); + exam.setStartDate(ZonedDateTime.now().minusMinutes(3)); + examRepository.save(exam); + + Set studentExamsBefore = studentExamRepository.findByExamId(exam.getId()); + assertThat(studentExamsBefore).isEmpty(); + + request.postWithoutLocation("/api/courses/" + course1.getId() + "/exams/" + exam.getId() + "/students/" + TEST_PREFIX + "student42", null, HttpStatus.OK, null); + + Set studentExamsAfter = studentExamRepository.findByExamId(exam.getId()); + assertThat(studentExamsAfter).hasSize(1); + } + @Test @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR") void testAddStudentToExam_testExam() throws Exception { diff --git a/src/test/java/de/tum/cit/aet/artemis/exam/service/ExamAccessServiceTest.java b/src/test/java/de/tum/cit/aet/artemis/exam/service/ExamAccessServiceTest.java index e7cce14f7eb0..5aa78b2af5be 100644 --- a/src/test/java/de/tum/cit/aet/artemis/exam/service/ExamAccessServiceTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/exam/service/ExamAccessServiceTest.java @@ -1,6 +1,7 @@ package de.tum.cit.aet.artemis.exam.service; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatNoException; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.InstanceOfAssertFactories.type; @@ -33,6 +34,9 @@ import de.tum.cit.aet.artemis.exam.repository.ExamUserRepository; import de.tum.cit.aet.artemis.exam.test_repository.StudentExamTestRepository; import de.tum.cit.aet.artemis.exam.util.ExamUtilService; +import de.tum.cit.aet.artemis.exercise.repository.ExerciseRepository; +import de.tum.cit.aet.artemis.quiz.domain.QuizExercise; +import de.tum.cit.aet.artemis.quiz.util.QuizExerciseFactory; import de.tum.cit.aet.artemis.shared.base.AbstractSpringIntegrationIndependentTest; class ExamAccessServiceTest extends AbstractSpringIntegrationIndependentTest { @@ -60,6 +64,9 @@ class ExamAccessServiceTest extends AbstractSpringIntegrationIndependentTest { @Autowired private ExamUtilService examUtilService; + @Autowired + private ExerciseRepository exerciseRepository; + private Course course1; private Course course2; @@ -123,11 +130,20 @@ void init() { examUser1 = examUserRepository.save(examUser1); testExam2.setExamUsers(Set.of(examUser1)); exerciseGroup1 = exam1.getExerciseGroups().getFirst(); + QuizExercise quiz = QuizExerciseFactory.generateQuizExerciseForExam(exerciseGroup1); + exerciseGroup1.addExercise(quiz); + exerciseRepository.save(quiz); exerciseGroup2 = exam2.getExerciseGroups().getFirst(); - studentExam1 = examUtilService.addStudentExam(exam1); + studentExam1 = examUtilService.addStudentExamWithUser(exam1, student1); studentExam2 = examUtilService.addStudentExam(exam2); studentExamForTestExam1 = examUtilService.addStudentExamForTestExam(testExam1, student1); studentExamForTestExam2 = examUtilService.addStudentExamForTestExam(testExam2, student1); + ExamUser examUser2 = new ExamUser(); + examUser2.setExam(exam1); + examUser2.setUser(student1); + examUser2 = examUserRepository.save(examUser2); + exam1.setExamUsers(Set.of(examUser2)); + exam1 = examRepository.save(exam1); } @Test @@ -332,10 +348,49 @@ void testCheckAndGetCourseAndExamAccessForConduction_examBelongsToCourse() { @Test @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER") - void testCheckAndGetCourseAndExamAccessForConduction_registeredUser() { - exam1.setExamUsers(Set.of()); + void testCheckAndGetCourseAndExamAccessForConduction_notRegisteredUser() { + assertThatThrownBy(() -> examAccessService.getExamInCourseElseThrow(course1.getId(), exam2.getId())).isInstanceOf(BadRequestAlertException.class); + } + + @Test + @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER") + void testCheckAndGetCourseAndExamAccessForConduction_registeredUser_noStudentExamPresent_examCanBeStarted() { + exam1.setStudentExams(Set.of()); + exam1.setStartDate(ZonedDateTime.now().plusMinutes(3)); + exam1.setEndDate(ZonedDateTime.now().plusMinutes(10)); + examRepository.save(exam1); + studentExamRepository.delete(studentExam1); + assertThatNoException().isThrownBy(() -> examAccessService.getExamInCourseElseThrow(course1.getId(), exam1.getId())); + } + + @Test + @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER") + void testCheckAndGetCourseAndExamAccessForConduction_registeredUser_noStudentExamPresent_examCannotBeStarted() { + exam1.setStudentExams(Set.of()); + exam1.setStartDate(ZonedDateTime.now().plusMinutes(7)); examRepository.save(exam1); - assertThatThrownBy(() -> examAccessService.getExamInCourseElseThrow(course1.getId(), exam1.getId())).isInstanceOf(BadRequestAlertException.class); + studentExamRepository.delete(studentExam1); + assertThatThrownBy(() -> examAccessService.getExamInCourseElseThrow(course1.getId(), exam1.getId())).asInstanceOf(type(BadRequestAlertException.class)) + .satisfies(error -> assertThat(error.getParameters().get("skipAlert")).isEqualTo(Boolean.TRUE)); + } + + @Test + @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER") + void testCheckAndGetCourseAndExamAccessForConduction_registeredUser_noStudentExamPresent_examHasEnded() { + exam1.setStudentExams(Set.of()); + exam1.setStartDate(ZonedDateTime.now().minusMinutes(10)); + exam1.setEndDate(ZonedDateTime.now().minusMinutes(7)); + examRepository.save(exam1); + studentExamRepository.delete(studentExam1); + assertThatThrownBy(() -> examAccessService.getExamInCourseElseThrow(course1.getId(), exam1.getId())).asInstanceOf(type(BadRequestAlertException.class)) + .satisfies(error -> assertThat(error.getParameters().get("skipAlert")).isEqualTo(Boolean.TRUE)); + } + + @Test + @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER") + void testCheckAndGetCourseAndExamAccessForConduction_registeredUser_studentExamPresent() { + StudentExam studentExam = examAccessService.getExamInCourseElseThrow(course1.getId(), exam1.getId()); + assertThat(studentExam.equals(studentExam1)).isEqualTo(true); } @Test @@ -355,14 +410,6 @@ void testGetExamInCourseElseThrow_noCourseAccess() { assertThatThrownBy(() -> examAccessService.getExamInCourseElseThrow(course1.getId(), testExam1.getId())).isInstanceOf(AccessForbiddenException.class); } - @Test - @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER") - void testGetExamInCourseElseThrow_realExam() { - assertThatThrownBy(() -> examAccessService.getExamInCourseElseThrow(course1.getId(), exam1.getId())).asInstanceOf(type(BadRequestAlertException.class)) - .satisfies(error -> assertThat(error.getParameters().get("skipAlert")).isEqualTo(Boolean.TRUE)); - - } - @Test @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER") void testGetExamInCourseElseThrow_notVisible() {