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

Adaptive learning: Fix competency progress update for exercise and lecture unit operations #8976

Merged
merged 21 commits into from
Jul 14, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
8db403a
Fix and test bug
JohannesStoehr Jul 5, 2024
6295673
Merge branch 'refs/heads/develop' into bugfix/adaptive-learning/updat…
JohannesStoehr Jul 5, 2024
8d186f3
Fix server tests
JohannesStoehr Jul 5, 2024
b1f0889
Merge branch 'refs/heads/develop' into bugfix/adaptive-learning/updat…
JohannesStoehr Jul 5, 2024
da168c8
Fix and test lecture deletion
JohannesStoehr Jul 5, 2024
66c17e6
Fix and server tests
JohannesStoehr Jul 5, 2024
2714cf4
Merge branch 'develop' into bugfix/adaptive-learning/update-progress-…
JohannesStoehr Jul 5, 2024
df5feea
Merge branch 'refs/heads/develop' into bugfix/adaptive-learning/updat…
JohannesStoehr Jul 7, 2024
55b6796
Review
JohannesStoehr Jul 7, 2024
cb3f297
Review and fix server tests
JohannesStoehr Jul 7, 2024
10b99ad
Improve assertions and fix client tests
JohannesStoehr Jul 7, 2024
f0e3863
Fix async actually being async
JohannesStoehr Jul 7, 2024
d7c741f
Improve mock verifications
JohannesStoehr Jul 7, 2024
d399c23
Flo
JohannesStoehr Jul 8, 2024
fb204cd
Merge branch 'refs/heads/develop' into bugfix/adaptive-learning/updat…
JohannesStoehr Jul 11, 2024
02a2a5a
Merge branch 'refs/heads/develop' into bugfix/adaptive-learning/updat…
JohannesStoehr Jul 12, 2024
68d0932
Merge branch 'refs/heads/develop' into bugfix/adaptive-learning/updat…
JohannesStoehr Jul 12, 2024
6deebbb
Fix merge conflict
JohannesStoehr Jul 13, 2024
f6f40a1
Remove unused methods
JohannesStoehr Jul 13, 2024
198445e
Fix query and only update necessary users
JohannesStoehr Jul 13, 2024
5ad41dd
Add JavaDoc
JohannesStoehr Jul 13, 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
4 changes: 2 additions & 2 deletions src/main/java/de/tum/in/www1/artemis/domain/Exercise.java
Original file line number Diff line number Diff line change
Expand Up @@ -1045,7 +1045,7 @@ private void validateExamExerciseIncludedInScoreCompletely() {
* Just setting the collections to {@code null} breaks the automatic orphan removal and change detection in the database.
*/
public void disconnectRelatedEntities() {
Stream.of(competencies, teams, gradingCriteria, studentParticipations, tutorParticipations, exampleSubmissions, attachments, posts, plagiarismCases)
.filter(Objects::nonNull).forEach(Collection::clear);
Stream.of(teams, gradingCriteria, studentParticipations, tutorParticipations, exampleSubmissions, attachments, posts, plagiarismCases).filter(Objects::nonNull)
.forEach(Collection::clear);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ default List<AttachmentUnit> findAllByLectureIdAndAttachmentTypeElseThrow(Long l
SELECT attachmentUnit
FROM AttachmentUnit attachmentUnit
LEFT JOIN FETCH attachmentUnit.slides slides
LEFT JOIN FETCH attachmentUnit.competencies
WHERE attachmentUnit.id = :attachmentUnitId
""")
AttachmentUnit findOneWithSlides(@Param("attachmentUnitId") long attachmentUnitId);
AttachmentUnit findOneWithSlidesAndCompetencies(@Param("attachmentUnitId") long attachmentUnitId);
JohannesStoehr marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import java.util.List;
import java.util.Optional;

import jakarta.validation.constraints.NotNull;

import org.springframework.context.annotation.Profile;
import org.springframework.data.jpa.repository.EntityGraph;
import org.springframework.data.jpa.repository.JpaSpecificationExecutor;
Expand All @@ -30,6 +32,14 @@ public interface FileUploadExerciseRepository extends ArtemisJpaRepository<FileU
""")
List<FileUploadExercise> findByCourseIdWithCategories(@Param("courseId") Long courseId);

@EntityGraph(type = LOAD, attributePaths = { "competencies" })
Optional<FileUploadExercise> findWithEagerCompetenciesById(Long exerciseId);

@EntityGraph(type = LOAD, attributePaths = { "teamAssignmentConfig", "categories", "competencies" })
Optional<FileUploadExercise> findWithEagerTeamAssignmentConfigAndCategoriesAndCompetenciesById(Long exerciseId);

@NotNull
default FileUploadExercise findWithEagerCompetenciesByIdElseThrow(Long exerciseId) {
return getValueElseThrow(findWithEagerCompetenciesById(exerciseId), exerciseId);
}
JohannesStoehr marked this conversation as resolved.
Show resolved Hide resolved
JohannesStoehr marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ public interface ModelingExerciseRepository extends ArtemisJpaRepository<Modelin
"plagiarismDetectionConfig" })
Optional<ModelingExercise> findWithEagerExampleSubmissionsAndCompetenciesAndPlagiarismDetectionConfigById(Long exerciseId);

@EntityGraph(type = LOAD, attributePaths = { "competencies" })
Optional<ModelingExercise> findWithEagerCompetenciesById(Long exerciseId);

@Query("""
SELECT modelingExercise
FROM ModelingExercise modelingExercise
Expand Down Expand Up @@ -112,4 +115,9 @@ default ModelingExercise findByIdWithExampleSubmissionsAndResultsAndPlagiarismDe
default ModelingExercise findByIdWithStudentParticipationsSubmissionsResultsElseThrow(long exerciseId) {
return findWithStudentParticipationsSubmissionsResultsById(exerciseId).orElseThrow(() -> new EntityNotFoundException("Modeling Exercise", exerciseId));
}

@NotNull
default ModelingExercise findWithEagerCompetenciesByIdElseThrow(long exerciseId) {
return getValueElseThrow(findWithEagerCompetenciesById(exerciseId), exerciseId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@

import static de.tum.in.www1.artemis.config.Constants.PROFILE_CORE;

import java.util.Optional;

import jakarta.validation.constraints.NotNull;

import org.springframework.context.annotation.Profile;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.query.Param;
import org.springframework.stereotype.Repository;

import de.tum.in.www1.artemis.domain.lecture.OnlineUnit;
Expand All @@ -15,4 +21,16 @@
@Repository
public interface OnlineUnitRepository extends ArtemisJpaRepository<OnlineUnit, Long> {

@Query("""
SELECT ou
FROM OnlineUnit ou
LEFT JOIN FETCH ou.competencies
WHERE ou.id = :onlineUnitId
""")
Optional<OnlineUnit> findByIdWithCompetencies(@Param("onlineUnitId") long onlineUnitId);

@NotNull
default OnlineUnit findByIdWithCompetenciesElseThrow(long onlineUnitId) {
return getValueElseThrow(findByIdWithCompetencies(onlineUnitId), onlineUnitId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ public interface ProgrammingExerciseRepository extends DynamicSpecificationRepos
@EntityGraph(type = LOAD, attributePaths = "auxiliaryRepositories")
Optional<ProgrammingExercise> findWithAuxiliaryRepositoriesById(long exerciseId);

@EntityGraph(type = LOAD, attributePaths = { "auxiliaryRepositories", "competencies" })
Optional<ProgrammingExercise> findWithAuxiliaryRepositoriesAndCompetenciesById(long exerciseId);

@EntityGraph(type = LOAD, attributePaths = "submissionPolicy")
Optional<ProgrammingExercise> findWithSubmissionPolicyById(long exerciseId);

Expand Down Expand Up @@ -520,6 +523,17 @@ default ProgrammingExercise findByIdWithAuxiliaryRepositoriesElseThrow(long prog
return findWithAuxiliaryRepositoriesById(programmingExerciseId).orElseThrow(() -> new EntityNotFoundException("Programming Exercise", programmingExerciseId));
}

/**
* Find a programming exercise with auxiliary repositories and competencies by its id and throw an EntityNotFoundException if it cannot be found
JohannesStoehr marked this conversation as resolved.
Show resolved Hide resolved
*
* @param programmingExerciseId of the programming exercise.
* @return The programming exercise related to the given id
*/
@NotNull
default ProgrammingExercise findByIdWithAuxiliaryRepositoriesAndCompetenciesElseThrow(long programmingExerciseId) throws EntityNotFoundException {
return getValueElseThrow(findWithAuxiliaryRepositoriesAndCompetenciesById(programmingExerciseId), programmingExerciseId);
}

/**
* Find a programming exercise with the submission policy by its id and throw an EntityNotFoundException if it cannot be found
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,12 @@ public interface QuizExerciseRepository extends ArtemisJpaRepository<QuizExercis
@EntityGraph(type = LOAD, attributePaths = { "quizQuestions" })
Optional<QuizExercise> findWithEagerQuestionsById(Long quizExerciseId);

@EntityGraph(type = LOAD, attributePaths = { "quizQuestions", "competencies" })
Optional<QuizExercise> findWithEagerQuestionsAndCompetenciesById(Long quizExerciseId);

@EntityGraph(type = LOAD, attributePaths = { "quizBatches" })
Optional<QuizExercise> findWithEagerBatchesById(Long quizExerciseId);

@NotNull
default QuizExercise findWithEagerQuestionsByIdOrElseThrow(Long quizExerciseId) {
return findWithEagerQuestionsById(quizExerciseId).orElseThrow(() -> new EntityNotFoundException("QuizExercise", quizExerciseId));
}

@NotNull
default QuizExercise findWithEagerBatchesByIdOrElseThrow(Long quizExerciseId) {
return findWithEagerBatchesById(quizExerciseId).orElseThrow(() -> new EntityNotFoundException("QuizExercise", quizExerciseId));
Expand Down Expand Up @@ -111,6 +109,17 @@ default QuizExercise findByIdWithQuestionsElseThrow(Long quizExerciseId) {
return findWithEagerQuestionsById(quizExerciseId).orElseThrow(() -> new EntityNotFoundException("Quiz Exercise", quizExerciseId));
}

/**
* Get one quiz exercise by id and eagerly load questions
*
* @param quizExerciseId the id of the entity
* @return the entity
*/
@NotNull
default QuizExercise findByIdWithQuestionsAndCompetenciesElseThrow(Long quizExerciseId) {
return getValueElseThrow(findWithEagerQuestionsAndCompetenciesById(quizExerciseId), quizExerciseId);
}

/**
* Get one quiz exercise by id and eagerly load batches
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ public interface TextExerciseRepository extends ArtemisJpaRepository<TextExercis
""")
List<TextExercise> findByCourseIdWithCategories(@Param("courseId") long courseId);

@EntityGraph(type = LOAD, attributePaths = { "competencies" })
Optional<TextExercise> findWithEagerCompetenciesById(long exerciseId);

@EntityGraph(type = LOAD, attributePaths = { "teamAssignmentConfig", "categories", "competencies" })
Optional<TextExercise> findWithEagerTeamAssignmentConfigAndCategoriesAndCompetenciesById(long exerciseId);

Expand Down Expand Up @@ -65,6 +68,11 @@ default TextExercise findWithGradingCriteriaByIdElseThrow(long exerciseId) {
return findWithGradingCriteriaById(exerciseId).orElseThrow(() -> new EntityNotFoundException("Text Exercise", exerciseId));
}

@NotNull
default TextExercise findWithEagerCompetenciesByIdElseThrow(long exerciseId) {
return getValueElseThrow(findWithEagerCompetenciesById(exerciseId), exerciseId);
}

@NotNull
default TextExercise findByIdWithExampleSubmissionsAndResultsElseThrow(long exerciseId) {
return findWithExampleSubmissionsAndResultsById(exerciseId).orElseThrow(() -> new EntityNotFoundException("Text Exercise", exerciseId));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,4 @@ public interface TextUnitRepository extends ArtemisJpaRepository<TextUnit, Long>
""")
Optional<TextUnit> findByIdWithCompetencies(@Param("textUnitId") Long textUnitId);

@Query("""
SELECT tu
FROM TextUnit tu
LEFT JOIN FETCH tu.competencies c
LEFT JOIN FETCH c.lectureUnits
WHERE tu.id = :textUnitId
""")
Optional<TextUnit> findByIdWithCompetenciesBidirectional(@Param("textUnitId") Long textUnitId);

}
Original file line number Diff line number Diff line change
Expand Up @@ -760,17 +760,6 @@ default User getUserWithGroupsAndAuthorities(@NotNull String username) {
return unwrapOptionalUser(user, username);
}

/**
* Get user with authorities with the username (i.e. user.getLogin() or principal.getName())
*
* @param username the username of the user who should be retrieved from the database
* @return the user that belongs to the given principal with eagerly loaded authorities
*/
default User getUserWithAuthorities(@NotNull String username) {
Optional<User> user = findOneWithAuthoritiesByLogin(username);
return unwrapOptionalUser(user, username);
}

/**
* Finds a single user with groups and authorities using the registration number
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@

import static de.tum.in.www1.artemis.config.Constants.PROFILE_CORE;

import java.util.Optional;

import jakarta.validation.constraints.NotNull;

import org.springframework.context.annotation.Profile;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.query.Param;
import org.springframework.stereotype.Repository;

import de.tum.in.www1.artemis.domain.lecture.VideoUnit;
Expand All @@ -15,4 +21,16 @@
@Repository
public interface VideoUnitRepository extends ArtemisJpaRepository<VideoUnit, Long> {

@Query("""
SELECT vu
FROM VideoUnit vu
LEFT JOIN FETCH vu.competencies
WHERE vu.id = :videoUnitId
""")
Optional<VideoUnit> findByIdWithCompetencies(@Param("videoUnitId") long videoUnitId);

@NotNull
default VideoUnit findByIdWithCompetenciesElseThrow(long videoUnitId) {
return getValueElseThrow(findByIdWithCompetencies(videoUnitId), videoUnitId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;

import org.apache.commons.io.FilenameUtils;
import org.springframework.context.annotation.Profile;
Expand All @@ -16,12 +17,14 @@

import de.tum.in.www1.artemis.domain.Attachment;
import de.tum.in.www1.artemis.domain.Lecture;
import de.tum.in.www1.artemis.domain.competency.Competency;
import de.tum.in.www1.artemis.domain.lecture.AttachmentUnit;
import de.tum.in.www1.artemis.domain.lecture.Slide;
import de.tum.in.www1.artemis.repository.AttachmentRepository;
import de.tum.in.www1.artemis.repository.AttachmentUnitRepository;
import de.tum.in.www1.artemis.repository.SlideRepository;
import de.tum.in.www1.artemis.repository.iris.IrisSettingsRepository;
import de.tum.in.www1.artemis.service.competency.CompetencyProgressService;
import de.tum.in.www1.artemis.service.connectors.pyris.PyrisWebhookService;
import de.tum.in.www1.artemis.web.rest.errors.BadRequestAlertException;

Expand All @@ -43,16 +46,19 @@ public class AttachmentUnitService {

private final Optional<IrisSettingsRepository> irisSettingsRepository;

private final CompetencyProgressService competencyProgressService;

public AttachmentUnitService(SlideRepository slideRepository, SlideSplitterService slideSplitterService, AttachmentUnitRepository attachmentUnitRepository,
AttachmentRepository attachmentRepository, FileService fileService, Optional<PyrisWebhookService> pyrisWebhookService,
Optional<IrisSettingsRepository> irisSettingsRepository) {
Optional<IrisSettingsRepository> irisSettingsRepository, CompetencyProgressService competencyProgressService) {
this.attachmentUnitRepository = attachmentUnitRepository;
this.attachmentRepository = attachmentRepository;
this.fileService = fileService;
this.slideSplitterService = slideSplitterService;
this.slideRepository = slideRepository;
this.pyrisWebhookService = pyrisWebhookService;
this.irisSettingsRepository = irisSettingsRepository;
this.competencyProgressService = competencyProgressService;
}

/**
Expand Down Expand Up @@ -98,9 +104,12 @@ public AttachmentUnit createAttachmentUnit(AttachmentUnit attachmentUnit, Attach
*/
public AttachmentUnit updateAttachmentUnit(AttachmentUnit existingAttachmentUnit, AttachmentUnit updateUnit, Attachment updateAttachment, MultipartFile updateFile,
boolean keepFilename) {
Set<Competency> existingCompetencies = existingAttachmentUnit.getCompetencies();

existingAttachmentUnit.setDescription(updateUnit.getDescription());
existingAttachmentUnit.setName(updateUnit.getName());
existingAttachmentUnit.setReleaseDate(updateUnit.getReleaseDate());
existingAttachmentUnit.setCompetencies(updateUnit.getCompetencies());
JohannesStoehr marked this conversation as resolved.
Show resolved Hide resolved

AttachmentUnit savedAttachmentUnit = attachmentUnitRepository.saveAndFlush(existingAttachmentUnit);

Expand Down Expand Up @@ -134,6 +143,11 @@ public AttachmentUnit updateAttachmentUnit(AttachmentUnit existingAttachmentUnit
pyrisWebhookService.get().autoUpdateAttachmentUnitsInPyris(savedAttachmentUnit.getLecture().getCourse().getId(), List.of(savedAttachmentUnit));
}
}

// Set the original competencies back to the attachment unit so that the progress can be updated correctly
JohannesStoehr marked this conversation as resolved.
Show resolved Hide resolved
existingAttachmentUnit.setCompetencies(existingCompetencies);
competencyProgressService.updateProgressForUpdatedLearningObject(existingAttachmentUnit, Optional.of(updateUnit));

return savedAttachmentUnit;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ private void deleteNotificationsOfCourse(Course course) {

private void deleteLecturesOfCourse(Course course) {
for (Lecture lecture : course.getLectures()) {
lectureService.delete(lecture);
lectureService.delete(lecture, false);
JohannesStoehr marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import de.tum.in.www1.artemis.domain.Exercise;
import de.tum.in.www1.artemis.domain.ProgrammingExercise;
import de.tum.in.www1.artemis.domain.TextExercise;
import de.tum.in.www1.artemis.domain.competency.Competency;
import de.tum.in.www1.artemis.domain.exam.StudentExam;
import de.tum.in.www1.artemis.domain.lecture.ExerciseUnit;
import de.tum.in.www1.artemis.domain.metis.conversation.Channel;
Expand All @@ -28,6 +29,7 @@
import de.tum.in.www1.artemis.repository.TutorParticipationRepository;
import de.tum.in.www1.artemis.repository.metis.conversation.ChannelRepository;
import de.tum.in.www1.artemis.repository.plagiarism.PlagiarismResultRepository;
import de.tum.in.www1.artemis.service.competency.CompetencyProgressService;
import de.tum.in.www1.artemis.service.metis.conversation.ChannelService;
import de.tum.in.www1.artemis.service.programming.ProgrammingExerciseService;
import de.tum.in.www1.artemis.service.quiz.QuizExerciseService;
Expand Down Expand Up @@ -70,11 +72,13 @@ public class ExerciseDeletionService {

private final ChannelService channelService;

private final CompetencyProgressService competencyProgressService;

public ExerciseDeletionService(ExerciseRepository exerciseRepository, ExerciseUnitRepository exerciseUnitRepository, ParticipationService participationService,
ProgrammingExerciseService programmingExerciseService, ModelingExerciseService modelingExerciseService, QuizExerciseService quizExerciseService,
TutorParticipationRepository tutorParticipationRepository, ExampleSubmissionService exampleSubmissionService, StudentExamRepository studentExamRepository,
LectureUnitService lectureUnitService, PlagiarismResultRepository plagiarismResultRepository, TextExerciseService textExerciseService,
ChannelRepository channelRepository, ChannelService channelService) {
ChannelRepository channelRepository, ChannelService channelService, CompetencyProgressService competencyProgressService) {
this.exerciseRepository = exerciseRepository;
this.participationService = participationService;
this.programmingExerciseService = programmingExerciseService;
Expand All @@ -89,6 +93,7 @@ public ExerciseDeletionService(ExerciseRepository exerciseRepository, ExerciseUn
this.textExerciseService = textExerciseService;
this.channelRepository = channelRepository;
this.channelService = channelService;
this.competencyProgressService = competencyProgressService;
}

/**
Expand Down Expand Up @@ -134,6 +139,7 @@ public void cleanup(Long exerciseId, boolean deleteRepositories) {
*/
public void delete(long exerciseId, boolean deleteStudentReposBuildPlans, boolean deleteBaseReposBuildPlans) {
var exercise = exerciseRepository.findWithCompetenciesByIdElseThrow(exerciseId);
Set<Competency> competencies = exercise.getCompetencies();
log.info("Request to delete {} with id {}", exercise.getClass().getSimpleName(), exerciseId);

long start = System.nanoTime();
Expand Down Expand Up @@ -163,7 +169,7 @@ public void delete(long exerciseId, boolean deleteStudentReposBuildPlans, boolea
plagiarismResultRepository.deletePlagiarismResultsByExerciseId(exerciseId);

// delete all participations belonging to this exercise, this will also delete submissions, results, feedback, complaints, etc.
participationService.deleteAllByExerciseId(exercise.getId(), deleteStudentReposBuildPlans, deleteStudentReposBuildPlans);
participationService.deleteAllByExercise(exercise, deleteStudentReposBuildPlans, deleteStudentReposBuildPlans, false);

// clean up the many-to-many relationship to avoid problems when deleting the entities but not the relationship table
exercise = exerciseRepository.findByIdWithEagerExampleSubmissionsElseThrow(exerciseId);
Expand Down Expand Up @@ -193,6 +199,8 @@ public void delete(long exerciseId, boolean deleteStudentReposBuildPlans, boolea
exercise = exerciseRepository.findByIdWithStudentParticipationsElseThrow(exerciseId);
exerciseRepository.delete(exercise);
}

competencies.forEach(competencyProgressService::updateProgressByCompetencyAsync);
}

/**
Expand Down Expand Up @@ -221,6 +229,6 @@ public void deletePlagiarismResultsAndParticipations(Exercise exercise) {
plagiarismResultRepository.deletePlagiarismResultsByExerciseId(exercise.getId());

// delete all participations belonging to this exercise, this will also delete submissions, results, feedback, complaints, etc.
participationService.deleteAllByExerciseId(exercise.getId(), true, true);
participationService.deleteAllByExercise(exercise, true, true, true);
}
}
Loading
Loading