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

Exam mode: Generate student exam on demand if student is registered for the exam #9123

Merged
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
e863572
merge methods generateTestExam and generateIndividualStudentExam into…
coolchock Jul 26, 2024
b75d860
generate a student exam on demand if student is registered for exam
coolchock Jul 26, 2024
dee2c03
Merge branch 'refs/heads/develop' into feature/exam-mode/generate-mis…
coolchock Aug 8, 2024
2bf5eb8
spotlessApply
coolchock Aug 8, 2024
06e3629
Merge branch 'refs/heads/develop' into feature/exam-mode/generate-mis…
coolchock Aug 13, 2024
350fd76
remove redundant test
coolchock Aug 14, 2024
1becb41
add test
coolchock Aug 14, 2024
bc5b509
remove jupiter Assertion
coolchock Aug 14, 2024
573417e
incorporate rabbit feedback
coolchock Aug 15, 2024
c92623f
Merge branch 'refs/heads/develop' into feature/exam-mode/generate-mis…
coolchock Aug 18, 2024
6480b12
Merge branch 'refs/heads/develop' into feature/exam-mode/generate-mis…
coolchock Aug 21, 2024
8c843a8
Merge branch 'refs/heads/develop' into feature/exam-mode/generate-mis…
coolchock Aug 25, 2024
cb1c80d
generate student exam only if exam can be started
coolchock Aug 25, 2024
7754dd3
add test cases
coolchock Aug 25, 2024
572e549
update comment
coolchock Aug 25, 2024
a4f1d9b
add comment
coolchock Aug 25, 2024
4637b65
Merge branch 'refs/heads/develop' into feature/exam-mode/generate-mis…
coolchock Aug 27, 2024
6c092fe
fix test
coolchock Aug 29, 2024
fe6dd14
add check if exam has ended
coolchock Aug 30, 2024
a9f564f
add test case for ended exam
coolchock Aug 30, 2024
9ca24a7
Merge branch 'refs/heads/develop' into feature/exam-mode/generate-mis…
coolchock Sep 2, 2024
c7bbddd
Merge branch 'refs/heads/develop' into feature/exam-mode/generate-mis…
coolchock Sep 3, 2024
6df9e20
Merge branch 'refs/heads/develop' into feature/exam-mode/generate-mis…
coolchock Sep 4, 2024
1988cf5
Merge branch 'refs/heads/develop' into feature/exam-mode/generate-mis…
coolchock Sep 12, 2024
1610bc1
resolve merge conflicts
coolchock Sep 12, 2024
9ecd237
Merge branch 'refs/heads/develop' into feature/exam-mode/generate-mis…
coolchock Sep 15, 2024
d0702e7
Merge branch 'refs/heads/develop' into feature/exam-mode/generate-mis…
coolchock Oct 1, 2024
bb6fc86
Merge branch 'develop' into feature/exam-mode/generate-missing-studen…
krusche Oct 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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));
coolchock marked this conversation as resolved.
Show resolved Hide resolved
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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,12 @@ public class ExamRegistrationService {

private static final boolean IS_TEST_RUN = false;

private final StudentExamService studentExamService;

coolchock marked this conversation as resolved.
Show resolved Hide resolved
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;
Expand All @@ -86,6 +89,7 @@ public ExamRegistrationService(ExamUserRepository examUserRepository, ExamReposi
this.authorizationCheckService = authorizationCheckService;
this.examUserRepository = examUserRepository;
this.examUserService = examUserService;
this.studentExamService = studentExamService;
}

/**
Expand Down Expand Up @@ -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.
coolchock marked this conversation as resolved.
Show resolved Hide resolved
*
* @param course the course containing the exam
* @param exam the exam for which we want to register a student
Expand All @@ -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);
}
coolchock marked this conversation as resolved.
Show resolved Hide resolved
}
else {
log.warn("Student {} is already registered for the exam {}", student.getLogin(), exam.getId());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<User> 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);
Expand All @@ -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<User> 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -23,9 +24,11 @@
import de.tum.in.www1.artemis.domain.User;
import de.tum.in.www1.artemis.domain.exam.Exam;
import de.tum.in.www1.artemis.domain.exam.ExamUser;
import de.tum.in.www1.artemis.domain.exam.StudentExam;
import de.tum.in.www1.artemis.domain.metis.conversation.Channel;
import de.tum.in.www1.artemis.repository.ExamRepository;
import de.tum.in.www1.artemis.repository.ExamUserRepository;
import de.tum.in.www1.artemis.repository.StudentExamRepository;
import de.tum.in.www1.artemis.repository.metis.conversation.ChannelRepository;
import de.tum.in.www1.artemis.service.dto.StudentDTO;
import de.tum.in.www1.artemis.service.exam.ExamRegistrationService;
Expand Down Expand Up @@ -59,6 +62,9 @@ class ExamRegistrationIntegrationTest extends AbstractSpringIntegrationLocalCILo
@Autowired
private ExamUtilService examUtilService;

@Autowired
private StudentExamRepository studentExamRepository;

private Course course1;

private Exam exam1;
Expand Down Expand Up @@ -107,6 +113,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<StudentExam> 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<StudentExam> studentExamsAfter = studentExamRepository.findByExamId(exam.getId());
assertThat(studentExamsAfter).hasSize(1);
coolchock marked this conversation as resolved.
Show resolved Hide resolved
}

@Test
@WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
void testAddStudentToExam_testExam() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package de.tum.in.www1.artemis.service.exam;

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;

Expand All @@ -22,9 +23,12 @@
import de.tum.in.www1.artemis.domain.exam.ExamUser;
import de.tum.in.www1.artemis.domain.exam.ExerciseGroup;
import de.tum.in.www1.artemis.domain.exam.StudentExam;
import de.tum.in.www1.artemis.domain.quiz.QuizExercise;
import de.tum.in.www1.artemis.exam.ExamUtilService;
import de.tum.in.www1.artemis.exercise.quiz.QuizExerciseFactory;
import de.tum.in.www1.artemis.repository.ExamRepository;
import de.tum.in.www1.artemis.repository.ExamUserRepository;
import de.tum.in.www1.artemis.repository.ExerciseRepository;
import de.tum.in.www1.artemis.repository.StudentExamRepository;
import de.tum.in.www1.artemis.repository.UserRepository;
import de.tum.in.www1.artemis.security.Role;
Expand Down Expand Up @@ -60,6 +64,9 @@ class ExamAccessServiceTest extends AbstractSpringIntegrationIndependentTest {
@Autowired
private ExamUtilService examUtilService;

@Autowired
private ExerciseRepository exerciseRepository;

private Course course1;

private Course course2;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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() {
Expand Down
Loading