From c907b0a7ee21f75eef787f0e7ecfe268879404d2 Mon Sep 17 00:00:00 2001 From: Simon Entholzer <33342534+SimonEntholzer@users.noreply.github.com> Date: Tue, 12 Nov 2024 23:41:38 +0100 Subject: [PATCH 1/8] Integrated code lifecycle: Add auxiliary repositories in exercise export and import (#9612) --- .../ProgrammingExerciseExportService.java | 1 + ...grammingExerciseImportFromFileService.java | 63 ++++++++++++++----- .../exercise-import-from-file.component.ts | 11 +++- 3 files changed, 57 insertions(+), 18 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseExportService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseExportService.java index 024d34348470..20d98af84bc8 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseExportService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseExportService.java @@ -186,6 +186,7 @@ protected void exportProblemStatementAndEmbeddedFilesAndExerciseDetails(Exercise if (exercise instanceof ProgrammingExercise programmingExercise) { // Used for a save typecast, this should always be true since this class only works with programming exercises. programmingExerciseTaskService.replaceTestIdsWithNames(programmingExercise); + programmingExercise.setAuxiliaryRepositories(auxiliaryRepositoryRepository.findByExerciseId(exercise.getId())); } super.exportProblemStatementAndEmbeddedFilesAndExerciseDetails(exercise, exportErrors, exportDir, pathsToBeZipped); } diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java index 3c8b1671cba9..784e29599185 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseImportFromFileService.java @@ -8,6 +8,8 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.stream.Stream; @@ -33,6 +35,7 @@ import de.tum.cit.aet.artemis.core.service.FileService; import de.tum.cit.aet.artemis.core.service.ProfileService; import de.tum.cit.aet.artemis.core.service.ZipFileService; +import de.tum.cit.aet.artemis.programming.domain.AuxiliaryRepository; import de.tum.cit.aet.artemis.programming.domain.ProgrammingExercise; import de.tum.cit.aet.artemis.programming.domain.Repository; import de.tum.cit.aet.artemis.programming.domain.RepositoryType; @@ -170,44 +173,72 @@ private void importRepositoriesFromFile(ProgrammingExercise newExercise, Path ba Repository templateRepo = gitService.getOrCheckoutRepository(new VcsRepositoryUri(newExercise.getTemplateRepositoryUri()), false); Repository solutionRepo = gitService.getOrCheckoutRepository(new VcsRepositoryUri(newExercise.getSolutionRepositoryUri()), false); Repository testRepo = gitService.getOrCheckoutRepository(new VcsRepositoryUri(newExercise.getTestRepositoryUri()), false); + List auxiliaryRepositories = new ArrayList<>(); + for (AuxiliaryRepository auxiliaryRepository : newExercise.getAuxiliaryRepositories()) { + auxiliaryRepositories.add(gitService.getOrCheckoutRepository(auxiliaryRepository.getVcsRepositoryUri(), false)); + } - copyImportedExerciseContentToRepositories(templateRepo, solutionRepo, testRepo, basePath); - replaceImportedExerciseShortName(Map.of(oldExerciseShortName, newExercise.getShortName()), templateRepo, solutionRepo, testRepo); + copyImportedExerciseContentToRepositories(templateRepo, solutionRepo, testRepo, auxiliaryRepositories, basePath); + replaceImportedExerciseShortName(Map.of(oldExerciseShortName, newExercise.getShortName()), List.of(solutionRepo, templateRepo, testRepo)); + replaceImportedExerciseShortName(Map.of(oldExerciseShortName, newExercise.getShortName()), auxiliaryRepositories); gitService.stageAllChanges(templateRepo); gitService.stageAllChanges(solutionRepo); gitService.stageAllChanges(testRepo); + for (Repository auxRepo : auxiliaryRepositories) { + gitService.stageAllChanges(auxRepo); + } gitService.commitAndPush(templateRepo, "Import template from file", true, user); gitService.commitAndPush(solutionRepo, "Import solution from file", true, user); gitService.commitAndPush(testRepo, "Import tests from file", true, user); + for (Repository auxRepo : auxiliaryRepositories) { + gitService.commitAndPush(auxRepo, "Import auxiliary repo from file", true, user); + } + } - private void replaceImportedExerciseShortName(Map replacements, Repository... repositories) { + private void replaceImportedExerciseShortName(Map replacements, List repositories) { for (Repository repository : repositories) { fileService.replaceVariablesInFileRecursive(repository.getLocalPath(), replacements, SHORT_NAME_REPLACEMENT_EXCLUSIONS); } } - private void copyImportedExerciseContentToRepositories(Repository templateRepo, Repository solutionRepo, Repository testRepo, Path basePath) throws IOException { + private void copyImportedExerciseContentToRepositories(Repository templateRepo, Repository solutionRepo, Repository testRepo, List auxiliaryRepositories, + Path basePath) throws IOException { repositoryService.deleteAllContentInRepository(templateRepo); repositoryService.deleteAllContentInRepository(solutionRepo); repositoryService.deleteAllContentInRepository(testRepo); - copyExerciseContentToRepository(templateRepo, RepositoryType.TEMPLATE, basePath); - copyExerciseContentToRepository(solutionRepo, RepositoryType.SOLUTION, basePath); - copyExerciseContentToRepository(testRepo, RepositoryType.TESTS, basePath); + for (Repository auxRepo : auxiliaryRepositories) { + repositoryService.deleteAllContentInRepository(auxRepo); + } + + copyExerciseContentToRepository(templateRepo, RepositoryType.TEMPLATE.getName(), basePath); + copyExerciseContentToRepository(solutionRepo, RepositoryType.SOLUTION.getName(), basePath); + copyExerciseContentToRepository(testRepo, RepositoryType.TESTS.getName(), basePath); + for (Repository auxRepo : auxiliaryRepositories) { + String[] parts = auxRepo.getLocalPath().toString().split("-"); + var auxRepoName = String.join("-", Arrays.copyOfRange(parts, 1, parts.length)); + copyExerciseContentToRepository(auxRepo, auxRepoName, basePath); + } } /** * Copies everything from the extracted zip file to the repository, except the .git folder * - * @param repository the repository to which the content should be copied - * @param repositoryType the type of the repository - * @param basePath the path to the extracted zip file + * @param repository the repository to which the content should be copied + * @param repoName the name of the repository + * @param basePath the path to the extracted zip file **/ - private void copyExerciseContentToRepository(Repository repository, RepositoryType repositoryType, Path basePath) throws IOException { - FileUtils.copyDirectory(retrieveRepositoryDirectoryPath(basePath, repositoryType.getName()).toFile(), repository.getLocalPath().toFile(), - new NotFileFilter(new NameFileFilter(".git"))); + private void copyExerciseContentToRepository(Repository repository, String repoName, Path basePath) throws IOException { + // @formatter:off + FileUtils.copyDirectory( + retrieveRepositoryDirectoryPath(basePath, repoName).toFile(), + repository.getLocalPath().toFile(), + new NotFileFilter(new NameFileFilter(".git")) + ); + // @formatter:on + try (var files = Files.walk(repository.getLocalPath())) { files.filter(file -> "gradlew".equals(file.getFileName().toString())).forEach(file -> file.toFile().setExecutable(true)); } @@ -242,17 +273,17 @@ private void checkRepositoryForTypeExists(Path path, RepositoryType repoType) th } } - private Path retrieveRepositoryDirectoryPath(Path dirPath, String repoType) { + private Path retrieveRepositoryDirectoryPath(Path dirPath, String repoName) { List result; try (Stream walk = Files.walk(dirPath)) { - result = walk.filter(Files::isDirectory).filter(file -> file.getFileName().toString().endsWith("-" + repoType)).toList(); + result = walk.filter(Files::isDirectory).filter(file -> file.getFileName().toString().endsWith("-" + repoName)).toList(); } catch (IOException e) { throw new BadRequestAlertException("Could not read the directory", "programmingExercise", "couldnotreaddirectory"); } if (result.size() != 1) { throw new IllegalArgumentException( - "There are either no or more than one sub-directories containing " + repoType + " in their name. Please make sure that there is exactly one."); + "There are either no or more than one sub-directories containing " + repoName + " in their name. Please make sure that there is exactly one."); } return result.getFirst(); diff --git a/src/main/webapp/app/exercises/shared/import/from-file/exercise-import-from-file.component.ts b/src/main/webapp/app/exercises/shared/import/from-file/exercise-import-from-file.component.ts index 688b7cd6fb44..79083e180ea2 100644 --- a/src/main/webapp/app/exercises/shared/import/from-file/exercise-import-from-file.component.ts +++ b/src/main/webapp/app/exercises/shared/import/from-file/exercise-import-from-file.component.ts @@ -50,10 +50,17 @@ export class ExerciseImportFromFileComponent implements OnInit { switch (this.exerciseType) { case ExerciseType.PROGRAMMING: this.exercise = JSON.parse(exerciseDetails as string) as ProgrammingExercise; + const progEx = this.exercise as ProgrammingExercise; // This is needed to make sure that old exported programming exercises can be imported - if (!(this.exercise as ProgrammingExercise).buildConfig) { - (this.exercise as ProgrammingExercise).buildConfig = copyBuildConfigFromExerciseJson(exerciseJson as ProgrammingExerciseBuildConfig); + if (!progEx.buildConfig) { + progEx.buildConfig = copyBuildConfigFromExerciseJson(exerciseJson as ProgrammingExerciseBuildConfig); } + if (progEx.auxiliaryRepositories) { + progEx.auxiliaryRepositories!.forEach((repo, index) => { + progEx.auxiliaryRepositories![index].id = undefined; + }); + } + this.exercise = progEx; break; default: this.alertService.error('artemisApp.exercise.importFromFile.notSupportedExerciseType', { From 65bc8904eccf54ff645ae3fb54ae1f1f15da47c0 Mon Sep 17 00:00:00 2001 From: Stephan Krusche Date: Tue, 12 Nov 2024 23:34:08 +0100 Subject: [PATCH 2/8] Programming exercises: Fix a performance issue with build log statistics --- .../core/repository/StatisticsRepository.java | 2 +- .../dto/BuildLogStatisticsDTO.java | 38 ++++++++++++++++++ .../BuildLogStatisticsEntryRepository.java | 39 +++++++++++++++++-- ...AndResultGitlabJenkinsIntegrationTest.java | 18 ++++----- 4 files changed, 83 insertions(+), 14 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/core/repository/StatisticsRepository.java b/src/main/java/de/tum/cit/aet/artemis/core/repository/StatisticsRepository.java index a39e6207f4bd..127dc533aef9 100644 --- a/src/main/java/de/tum/cit/aet/artemis/core/repository/StatisticsRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/core/repository/StatisticsRepository.java @@ -121,7 +121,7 @@ OR EXISTS (SELECT c FROM Course c WHERE submission.participation.exercise.course * * @param startDate the minimum submission date * @param endDate the maximum submission date - * @return a list of active users + * @return a count of active users */ @Query(""" SELECT COUNT(DISTINCT p.student.id) diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/dto/BuildLogStatisticsDTO.java b/src/main/java/de/tum/cit/aet/artemis/programming/dto/BuildLogStatisticsDTO.java index 00908d9d94d1..8103861bbc41 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/dto/BuildLogStatisticsDTO.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/dto/BuildLogStatisticsDTO.java @@ -1,8 +1,46 @@ package de.tum.cit.aet.artemis.programming.dto; +import jakarta.validation.constraints.NotNull; + import com.fasterxml.jackson.annotation.JsonInclude; @JsonInclude(JsonInclude.Include.NON_EMPTY) public record BuildLogStatisticsDTO(Long buildCount, Double agentSetupDuration, Double testDuration, Double scaDuration, Double totalJobDuration, Double dependenciesDownloadedCount) { + + @NotNull + @Override + public Long buildCount() { + return buildCount != null ? buildCount : 0; + } + + @NotNull + @Override + public Double agentSetupDuration() { + return agentSetupDuration != null ? agentSetupDuration : 0.0; + } + + @NotNull + @Override + public Double testDuration() { + return testDuration != null ? testDuration : 0.0; + } + + @NotNull + @Override + public Double scaDuration() { + return scaDuration != null ? scaDuration : 0.0; + } + + @NotNull + @Override + public Double totalJobDuration() { + return totalJobDuration != null ? totalJobDuration : 0.0; + } + + @NotNull + @Override + public Double dependenciesDownloadedCount() { + return dependenciesDownloadedCount != null ? dependenciesDownloadedCount : 0.0; + } } diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/repository/BuildLogStatisticsEntryRepository.java b/src/main/java/de/tum/cit/aet/artemis/programming/repository/BuildLogStatisticsEntryRepository.java index b653d693bbf2..ec203b37eadd 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/repository/BuildLogStatisticsEntryRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/repository/BuildLogStatisticsEntryRepository.java @@ -23,6 +23,20 @@ @Repository public interface BuildLogStatisticsEntryRepository extends ArtemisJpaRepository { + @Query(""" + SELECT new de.tum.cit.aet.artemis.programming.dto.BuildLogStatisticsDTO( + COUNT(b.id), + AVG(b.agentSetupDuration), + AVG(b.testDuration), + AVG(b.scaDuration), + AVG(b.totalJobDuration), + AVG(b.dependenciesDownloadedCount) + ) + FROM BuildLogStatisticsEntry b + WHERE b.programmingSubmission.participation.exercise = :exercise + """) + BuildLogStatisticsDTO findAverageStudentBuildLogStatistics(@Param("exercise") ProgrammingExercise exercise); + @Query(""" SELECT new de.tum.cit.aet.artemis.programming.dto.BuildLogStatisticsDTO( COUNT(b.id), @@ -35,16 +49,33 @@ public interface BuildLogStatisticsEntryRepository extends ArtemisJpaRepository< FROM BuildLogStatisticsEntry b LEFT JOIN b.programmingSubmission s LEFT JOIN s.participation p - WHERE p.exercise = :exercise - OR p.id = :templateParticipationId + WHERE p.id = :templateParticipationId OR p.id = :solutionParticipationId """) - BuildLogStatisticsDTO findAverageBuildLogStatistics(@Param("exercise") ProgrammingExercise exercise, @Param("templateParticipationId") Long templateParticipationId, + BuildLogStatisticsDTO findAverageExerciseBuildLogStatistics(@Param("templateParticipationId") Long templateParticipationId, @Param("solutionParticipationId") Long solutionParticipationId); + /** + * Find the average build log statistics for the given exercise. If the exercise has a template or solution participation, the statistics are also calculated for these + * NOTE: we cannot calculate this within one query, this would be way too slow, therefore, we split it into multiple queries and combine the result + * + * @param exercise the exercise for which the statistics should be calculated + * @return the average build log statistics + */ default BuildLogStatisticsDTO findAverageBuildLogStatistics(ProgrammingExercise exercise) { - return findAverageBuildLogStatistics(exercise, exercise.getTemplateParticipation() != null ? exercise.getTemplateParticipation().getId() : null, + var studentStatistics = findAverageStudentBuildLogStatistics(exercise); + var exerciseStatistics = findAverageExerciseBuildLogStatistics(exercise.getTemplateParticipation() != null ? exercise.getTemplateParticipation().getId() : null, exercise.getSolutionParticipation() != null ? exercise.getSolutionParticipation().getId() : null); + // combine both based on the count + var studentCount = studentStatistics.buildCount(); + var exerciseCount = exerciseStatistics.buildCount(); + return new BuildLogStatisticsDTO(studentCount + exerciseCount, + studentStatistics.agentSetupDuration() * studentCount + exerciseStatistics.agentSetupDuration() * exerciseCount, + studentStatistics.testDuration() * studentCount + exerciseStatistics.testDuration() * exerciseCount, + studentStatistics.scaDuration() * studentCount + exerciseStatistics.scaDuration() * exerciseCount, + studentStatistics.totalJobDuration() * studentCount + exerciseStatistics.totalJobDuration() * exerciseCount, + studentStatistics.dependenciesDownloadedCount() * studentCount + exerciseStatistics.dependenciesDownloadedCount() * exerciseCount); + } @Transactional // ok because of delete diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingSubmissionAndResultGitlabJenkinsIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingSubmissionAndResultGitlabJenkinsIntegrationTest.java index 102643f76237..89dc295175d7 100644 --- a/src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingSubmissionAndResultGitlabJenkinsIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingSubmissionAndResultGitlabJenkinsIntegrationTest.java @@ -117,7 +117,7 @@ void shouldExtractBuildLogAnalytics_noSca() throws Exception { assertThat(statistics.buildCount()).isEqualTo(1); assertThat(statistics.agentSetupDuration()).isEqualTo(90); assertThat(statistics.testDuration()).isEqualTo(10); - assertThat(statistics.scaDuration()).isNull(); + assertThat(statistics.scaDuration()).isEqualTo(0.0); assertThat(statistics.totalJobDuration()).isEqualTo(110); assertThat(statistics.dependenciesDownloadedCount()).isEqualTo(1); } @@ -156,11 +156,11 @@ void shouldExtractBuildLogAnalytics_noSca_gradle() throws Exception { var statistics = buildLogStatisticsEntryRepository.findAverageBuildLogStatistics(exercise); assertThat(statistics.buildCount()).isEqualTo(1); - assertThat(statistics.agentSetupDuration()).isNull(); + assertThat(statistics.agentSetupDuration()).isEqualTo(0.0); assertThat(statistics.testDuration()).isEqualTo(20); - assertThat(statistics.scaDuration()).isNull(); + assertThat(statistics.scaDuration()).isEqualTo(0.0); assertThat(statistics.totalJobDuration()).isEqualTo(20); - assertThat(statistics.dependenciesDownloadedCount()).isNull(); + assertThat(statistics.dependenciesDownloadedCount()).isEqualTo(0.0); } @Test @@ -209,11 +209,11 @@ void shouldExtractBuildLogAnalytics_unsupportedProgrammingLanguage() throws Exce var statistics = buildLogStatisticsEntryRepository.findAverageBuildLogStatistics(exercise); // Should not extract any statistics assertThat(statistics.buildCount()).isZero(); - assertThat(statistics.agentSetupDuration()).isNull(); - assertThat(statistics.testDuration()).isNull(); - assertThat(statistics.scaDuration()).isNull(); - assertThat(statistics.totalJobDuration()).isNull(); - assertThat(statistics.dependenciesDownloadedCount()).isNull(); + assertThat(statistics.agentSetupDuration()).isEqualTo(0.0); + assertThat(statistics.testDuration()).isEqualTo(0.0); + assertThat(statistics.scaDuration()).isEqualTo(0.0); + assertThat(statistics.totalJobDuration()).isEqualTo(0.0); + assertThat(statistics.dependenciesDownloadedCount()).isEqualTo(0.0); } private static Stream shouldSaveBuildLogsOnStudentParticipationArguments() { From 749b301b7c370b514f47d6a05895b6b0efab6e89 Mon Sep 17 00:00:00 2001 From: Stephan Krusche Date: Tue, 12 Nov 2024 23:46:47 +0100 Subject: [PATCH 3/8] Programming exercise: Speed up version control access log queries with a database index --- .../repository/VcsAccessLogRepository.java | 2 +- .../changelog/20241112123600_changelog.xml | 17 +++++++++++++++++ src/main/resources/config/liquibase/master.xml | 1 + 3 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 src/main/resources/config/liquibase/changelog/20241112123600_changelog.xml diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java b/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java index 628019f34eaa..76722cfd5eea 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/repository/VcsAccessLogRepository.java @@ -39,7 +39,7 @@ public interface VcsAccessLogRepository extends ArtemisJpaRepository findNewestByParticipationId(@Param("participationId") long participationId); /** diff --git a/src/main/resources/config/liquibase/changelog/20241112123600_changelog.xml b/src/main/resources/config/liquibase/changelog/20241112123600_changelog.xml new file mode 100644 index 000000000000..c49a4b3c480a --- /dev/null +++ b/src/main/resources/config/liquibase/changelog/20241112123600_changelog.xml @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + diff --git a/src/main/resources/config/liquibase/master.xml b/src/main/resources/config/liquibase/master.xml index 077b50e53e89..6a06b398b783 100644 --- a/src/main/resources/config/liquibase/master.xml +++ b/src/main/resources/config/liquibase/master.xml @@ -33,6 +33,7 @@ + From f2d7aaafd4d70a3f699a5606f6827731f36e95da Mon Sep 17 00:00:00 2001 From: Stephan Krusche Date: Tue, 12 Nov 2024 23:47:30 +0100 Subject: [PATCH 4/8] Adaptive learning: Fix an issue with competency progress calculation --- .../artemis/atlas/repository/CourseCompetencyRepository.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/atlas/repository/CourseCompetencyRepository.java b/src/main/java/de/tum/cit/aet/artemis/atlas/repository/CourseCompetencyRepository.java index fee3ca1e82f2..bba103f436d9 100644 --- a/src/main/java/de/tum/cit/aet/artemis/atlas/repository/CourseCompetencyRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/atlas/repository/CourseCompetencyRepository.java @@ -126,7 +126,7 @@ CASE WHEN TYPE(e) = ProgrammingExercise THEN TRUE ELSE FALSE END, LEFT JOIN TeamScore tS ON tS.exercise = e AND :user MEMBER OF tS.team.students WHERE c.id = :competencyId AND e IS NOT NULL - GROUP BY e.id, e.maxPoints, e.difficulty, TYPE(e), sS.lastScore, tS.lastScore, sS.lastPoints, tS.lastPoints, sS.lastModifiedDate, tS.lastModifiedDate + GROUP BY e.id, e.maxPoints, e.difficulty, TYPE(e), el.weight, sS.lastScore, tS.lastScore, sS.lastPoints, tS.lastPoints, sS.lastModifiedDate, tS.lastModifiedDate """) Set findAllExerciseInfoByCompetencyIdAndUser(@Param("competencyId") long competencyId, @Param("user") User user); From f4855278b13183fbd47bbe0ad6785541b8aaffde Mon Sep 17 00:00:00 2001 From: Florian Glombik <63976129+florian-glombik@users.noreply.github.com> Date: Tue, 12 Nov 2024 23:51:09 +0100 Subject: [PATCH 5/8] Lectures: Fix an issue when saving lecture units in guided mode without linked competency (#9758) --- .../lecture/service/LectureUnitService.java | 2 +- .../artemis/lecture/LectureUnitServiceTest.java | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/lecture/service/LectureUnitService.java b/src/main/java/de/tum/cit/aet/artemis/lecture/service/LectureUnitService.java index 793752998f29..bda282499738 100644 --- a/src/main/java/de/tum/cit/aet/artemis/lecture/service/LectureUnitService.java +++ b/src/main/java/de/tum/cit/aet/artemis/lecture/service/LectureUnitService.java @@ -253,7 +253,7 @@ public T saveWithCompetencyLinks(T lectureUnit, Function T savedLectureUnit = saveFunction.apply(lectureUnit); - if (Hibernate.isInitialized(links) && !links.isEmpty()) { + if (Hibernate.isInitialized(links) && links != null && !links.isEmpty()) { savedLectureUnit.setCompetencyLinks(links); reconnectCompetencyLectureUnitLinks(savedLectureUnit); savedLectureUnit.setCompetencyLinks(new HashSet<>(competencyLectureUnitLinkRepository.saveAll(links))); diff --git a/src/test/java/de/tum/cit/aet/artemis/lecture/LectureUnitServiceTest.java b/src/test/java/de/tum/cit/aet/artemis/lecture/LectureUnitServiceTest.java index 2a79f139fc96..882e19f57822 100644 --- a/src/test/java/de/tum/cit/aet/artemis/lecture/LectureUnitServiceTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/lecture/LectureUnitServiceTest.java @@ -11,6 +11,7 @@ import de.tum.cit.aet.artemis.core.domain.User; import de.tum.cit.aet.artemis.core.user.util.UserUtilService; +import de.tum.cit.aet.artemis.lecture.domain.AttachmentUnit; import de.tum.cit.aet.artemis.lecture.domain.Lecture; import de.tum.cit.aet.artemis.lecture.domain.LectureUnit; import de.tum.cit.aet.artemis.lecture.domain.LectureUnitCompletion; @@ -87,4 +88,18 @@ void testUncompleteAllUnits() { assertThat(lectureUnitCompletionRepository.findByLectureUnitIdAndUserId(unit1.getId(), student1.getId())).isEmpty(); assertThat(lectureUnitCompletionRepository.findByLectureUnitIdAndUserId(unit2.getId(), student1.getId())).isEmpty(); } + + @Test + void testSaveWithCompetencyLinksWithNullLinks() { + AttachmentUnit lectureUnit = new AttachmentUnit(); + lectureUnit.setCompetencyLinks(null); + + LectureUnit savedLectureUnit = lectureUnitService.saveWithCompetencyLinks(lectureUnit, unit -> { + unit.setId(1L); + return unit; + }); + + assertThat(savedLectureUnit).isNotNull(); + assertThat(savedLectureUnit.getCompetencyLinks()).isEmpty(); + } } From 1b504663f1357210db413bb84c439b9d1d82007d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20St=C3=B6hr?= <38322605+JohannesStoehr@users.noreply.github.com> Date: Tue, 12 Nov 2024 23:51:34 +0100 Subject: [PATCH 6/8] Adaptive learning: Fix linking attachment units to competencies (#9739) --- .../lecture/repository/AttachmentUnitRepository.java | 7 ++++++- .../aet/artemis/lecture/service/AttachmentUnitService.java | 1 + .../aet/artemis/lecture/web/AttachmentUnitResource.java | 4 ++-- .../aet/artemis/lecture/AttachmentUnitIntegrationTest.java | 3 +++ 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/lecture/repository/AttachmentUnitRepository.java b/src/main/java/de/tum/cit/aet/artemis/lecture/repository/AttachmentUnitRepository.java index 61f268d75b95..f43d8a101af5 100644 --- a/src/main/java/de/tum/cit/aet/artemis/lecture/repository/AttachmentUnitRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/lecture/repository/AttachmentUnitRepository.java @@ -3,6 +3,7 @@ import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_CORE; import java.util.List; +import java.util.Optional; import jakarta.validation.constraints.NotNull; @@ -62,5 +63,9 @@ default List findAllByLectureIdAndAttachmentTypeElseThrow(Long l LEFT JOIN FETCH cl.competency WHERE attachmentUnit.id = :attachmentUnitId """) - AttachmentUnit findOneWithSlidesAndCompetencies(@Param("attachmentUnitId") long attachmentUnitId); + Optional findWithSlidesAndCompetenciesById(@Param("attachmentUnitId") long attachmentUnitId); + + default AttachmentUnit findWithSlidesAndCompetenciesByIdElseThrow(long attachmentUnitId) { + return getValueElseThrow(findWithSlidesAndCompetenciesById(attachmentUnitId), attachmentUnitId); + } } diff --git a/src/main/java/de/tum/cit/aet/artemis/lecture/service/AttachmentUnitService.java b/src/main/java/de/tum/cit/aet/artemis/lecture/service/AttachmentUnitService.java index e86d7c22e7f5..83d3ffdf6751 100644 --- a/src/main/java/de/tum/cit/aet/artemis/lecture/service/AttachmentUnitService.java +++ b/src/main/java/de/tum/cit/aet/artemis/lecture/service/AttachmentUnitService.java @@ -117,6 +117,7 @@ public AttachmentUnit updateAttachmentUnit(AttachmentUnit existingAttachmentUnit existingAttachmentUnit.setDescription(updateUnit.getDescription()); existingAttachmentUnit.setName(updateUnit.getName()); existingAttachmentUnit.setReleaseDate(updateUnit.getReleaseDate()); + existingAttachmentUnit.setCompetencyLinks(updateUnit.getCompetencyLinks()); AttachmentUnit savedAttachmentUnit = lectureUnitService.saveWithCompetencyLinks(existingAttachmentUnit, attachmentUnitRepository::saveAndFlush); diff --git a/src/main/java/de/tum/cit/aet/artemis/lecture/web/AttachmentUnitResource.java b/src/main/java/de/tum/cit/aet/artemis/lecture/web/AttachmentUnitResource.java index 0c7c2996e162..83aa9f8ebefb 100644 --- a/src/main/java/de/tum/cit/aet/artemis/lecture/web/AttachmentUnitResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/lecture/web/AttachmentUnitResource.java @@ -100,7 +100,7 @@ public AttachmentUnitResource(AttachmentUnitRepository attachmentUnitRepository, @EnforceAtLeastEditor public ResponseEntity getAttachmentUnit(@PathVariable Long attachmentUnitId, @PathVariable Long lectureId) { log.debug("REST request to get AttachmentUnit : {}", attachmentUnitId); - AttachmentUnit attachmentUnit = attachmentUnitRepository.findByIdElseThrow(attachmentUnitId); + AttachmentUnit attachmentUnit = attachmentUnitRepository.findWithSlidesAndCompetenciesByIdElseThrow(attachmentUnitId); checkAttachmentUnitCourseAndLecture(attachmentUnit, lectureId); authorizationCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.EDITOR, attachmentUnit.getLecture().getCourse(), null); @@ -125,7 +125,7 @@ public ResponseEntity updateAttachmentUnit(@PathVariable Long le @RequestPart Attachment attachment, @RequestPart(required = false) MultipartFile file, @RequestParam(defaultValue = "false") boolean keepFilename, @RequestParam(value = "notificationText", required = false) String notificationText) { log.debug("REST request to update an attachment unit : {}", attachmentUnit); - AttachmentUnit existingAttachmentUnit = attachmentUnitRepository.findOneWithSlidesAndCompetencies(attachmentUnitId); + AttachmentUnit existingAttachmentUnit = attachmentUnitRepository.findWithSlidesAndCompetenciesByIdElseThrow(attachmentUnitId); checkAttachmentUnitCourseAndLecture(existingAttachmentUnit, lectureId); authorizationCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.EDITOR, existingAttachmentUnit.getLecture().getCourse(), null); diff --git a/src/test/java/de/tum/cit/aet/artemis/lecture/AttachmentUnitIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/lecture/AttachmentUnitIntegrationTest.java index d127e1d40db4..929d46b4b5bb 100644 --- a/src/test/java/de/tum/cit/aet/artemis/lecture/AttachmentUnitIntegrationTest.java +++ b/src/test/java/de/tum/cit/aet/artemis/lecture/AttachmentUnitIntegrationTest.java @@ -278,6 +278,7 @@ void updateAttachmentUnit_asInstructor_shouldUpdateAttachmentUnit() throws Excep attachment = attachmentRepository.findById(attachment.getId()).orElseThrow(); assertThat(attachmentUnit2.getAttachment()).isEqualTo(attachment); assertThat(attachment.getAttachmentUnit()).isEqualTo(attachmentUnit2); + assertThat(attachmentUnit1.getCompetencyLinks()).anyMatch(link -> link.getCompetency().getId().equals(competency.getId())); verify(competencyProgressService, timeout(1000).times(1)).updateProgressForUpdatedLearningObjectAsync(eq(attachmentUnit), eq(Optional.of(attachmentUnit))); } @@ -334,6 +335,7 @@ void getAttachmentUnit_correctId_shouldReturnAttachmentUnit() throws Exception { this.attachment.setAttachmentUnit(this.attachmentUnit); this.attachment = attachmentRepository.save(attachment); this.attachmentUnit = this.attachmentUnitRepository.save(this.attachmentUnit); + competencyUtilService.linkLectureUnitToCompetency(competency, attachmentUnit); // 1. check the database call directly this.attachmentUnit = this.attachmentUnitRepository.findByIdElseThrow(this.attachmentUnit.getId()); @@ -342,6 +344,7 @@ void getAttachmentUnit_correctId_shouldReturnAttachmentUnit() throws Exception { // 2. check the REST call this.attachmentUnit = request.get("/api/lectures/" + lecture1.getId() + "/attachment-units/" + this.attachmentUnit.getId(), HttpStatus.OK, AttachmentUnit.class); assertThat(this.attachmentUnit.getAttachment()).isEqualTo(this.attachment); + assertThat(this.attachmentUnit.getCompetencyLinks()).anyMatch(link -> link.getCompetency().getId().equals(competency.getId())); } @Test From 70ccfc361adf77ff0fa64a2e982796047cbbb132 Mon Sep 17 00:00:00 2001 From: Stephan Krusche Date: Wed, 13 Nov 2024 09:38:41 +0100 Subject: [PATCH 7/8] Development: Fix server tests related to build log statistics --- .../BuildLogStatisticsEntryRepository.java | 15 ++++++++------- .../util/ProgrammingExerciseTestService.java | 12 ++++++------ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/repository/BuildLogStatisticsEntryRepository.java b/src/main/java/de/tum/cit/aet/artemis/programming/repository/BuildLogStatisticsEntryRepository.java index ec203b37eadd..11f34865096b 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/repository/BuildLogStatisticsEntryRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/repository/BuildLogStatisticsEntryRepository.java @@ -66,15 +66,16 @@ default BuildLogStatisticsDTO findAverageBuildLogStatistics(ProgrammingExercise var studentStatistics = findAverageStudentBuildLogStatistics(exercise); var exerciseStatistics = findAverageExerciseBuildLogStatistics(exercise.getTemplateParticipation() != null ? exercise.getTemplateParticipation().getId() : null, exercise.getSolutionParticipation() != null ? exercise.getSolutionParticipation().getId() : null); - // combine both based on the count + // build the average of two values based on the count var studentCount = studentStatistics.buildCount(); var exerciseCount = exerciseStatistics.buildCount(); - return new BuildLogStatisticsDTO(studentCount + exerciseCount, - studentStatistics.agentSetupDuration() * studentCount + exerciseStatistics.agentSetupDuration() * exerciseCount, - studentStatistics.testDuration() * studentCount + exerciseStatistics.testDuration() * exerciseCount, - studentStatistics.scaDuration() * studentCount + exerciseStatistics.scaDuration() * exerciseCount, - studentStatistics.totalJobDuration() * studentCount + exerciseStatistics.totalJobDuration() * exerciseCount, - studentStatistics.dependenciesDownloadedCount() * studentCount + exerciseStatistics.dependenciesDownloadedCount() * exerciseCount); + var count = studentCount + exerciseCount; + return new BuildLogStatisticsDTO(count, + count == 0 ? 0.0 : (studentStatistics.agentSetupDuration() * studentCount + exerciseStatistics.agentSetupDuration() * exerciseCount) / count, + count == 0 ? 0.0 : (studentStatistics.testDuration() * studentCount + exerciseStatistics.testDuration() * exerciseCount) / count, + count == 0 ? 0.0 : (studentStatistics.scaDuration() * studentCount + exerciseStatistics.scaDuration() * exerciseCount) / count, + count == 0 ? 0.0 : (studentStatistics.totalJobDuration() * studentCount + exerciseStatistics.totalJobDuration() * exerciseCount) / count, + count == 0 ? 0.0 : (studentStatistics.dependenciesDownloadedCount() * studentCount + exerciseStatistics.dependenciesDownloadedCount() * exerciseCount) / count); } diff --git a/src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseTestService.java b/src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseTestService.java index b8963f3decae..21b133700a9e 100644 --- a/src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseTestService.java +++ b/src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseTestService.java @@ -2630,12 +2630,12 @@ public void buildLogStatistics_noStatistics() throws Exception { exercise.setBuildConfig(programmingExerciseBuildConfigRepository.save(exercise.getBuildConfig())); exercise = programmingExerciseRepository.save(exercise); var statistics = request.get("/api/programming-exercises/" + exercise.getId() + "/build-log-statistics", HttpStatus.OK, BuildLogStatisticsDTO.class); - assertThat(statistics.buildCount()).isZero(); - assertThat(statistics.agentSetupDuration()).isNull(); - assertThat(statistics.testDuration()).isNull(); - assertThat(statistics.scaDuration()).isNull(); - assertThat(statistics.totalJobDuration()).isNull(); - assertThat(statistics.dependenciesDownloadedCount()).isNull(); + assertThat(statistics.buildCount()).isEqualTo(0); + assertThat(statistics.agentSetupDuration()).isEqualTo(0); + assertThat(statistics.testDuration()).isEqualTo(0); + assertThat(statistics.scaDuration()).isEqualTo(0); + assertThat(statistics.totalJobDuration()).isEqualTo(0); + assertThat(statistics.dependenciesDownloadedCount()).isEqualTo(0); } // TEST From 054476c7d8bcc12a21e35b4b1760d3c007df3a95 Mon Sep 17 00:00:00 2001 From: Stephan Krusche Date: Wed, 13 Nov 2024 11:18:34 +0100 Subject: [PATCH 8/8] Development: Improve slow query performance (#9727) --- .../ParticipantScoreRepository.java | 4 - .../StandardizedCompetencyService.java | 1 + .../aet/artemis/core/dto/CourseGroupsDTO.java | 7 + .../core/repository/CourseRepository.java | 28 ++-- .../artemis/core/service/CourseService.java | 1 + .../aet/artemis/core/web/AccountResource.java | 4 - .../aet/artemis/core/web/CourseResource.java | 133 +++++++----------- .../aet/artemis/core/web/FileResource.java | 46 +++--- .../core/web/admin/AdminCourseResource.java | 19 +-- .../StudentParticipationRepository.java | 25 ++-- .../exercise/service/SubmissionService.java | 19 ++- .../service/FileUploadSubmissionService.java | 8 +- .../web/FileUploadSubmissionResource.java | 14 +- .../cit/aet/artemis/lti/web/LtiResource.java | 51 ++++++- .../ModelingSubmissionRepository.java | 2 +- .../service/ModelingSubmissionService.java | 34 ++--- .../ProgrammingExerciseTaskService.java | 2 +- .../web/ProgrammingSubmissionResource.java | 4 +- .../repository/TextSubmissionRepository.java | 6 +- .../text/service/TextSubmissionService.java | 10 +- .../text/web/TextAssessmentResource.java | 8 +- .../text/web/TextSubmissionResource.java | 8 +- 22 files changed, 235 insertions(+), 199 deletions(-) create mode 100644 src/main/java/de/tum/cit/aet/artemis/core/dto/CourseGroupsDTO.java diff --git a/src/main/java/de/tum/cit/aet/artemis/assessment/repository/ParticipantScoreRepository.java b/src/main/java/de/tum/cit/aet/artemis/assessment/repository/ParticipantScoreRepository.java index 14598bad30b1..24636efc2330 100644 --- a/src/main/java/de/tum/cit/aet/artemis/assessment/repository/ParticipantScoreRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/assessment/repository/ParticipantScoreRepository.java @@ -44,10 +44,6 @@ public interface ParticipantScoreRepository extends ArtemisJpaRepository findAllOutdated(); - @Override - @EntityGraph(type = LOAD, attributePaths = { "exercise", "lastResult", "lastRatedResult" }) - List findAll(); - @EntityGraph(type = LOAD, attributePaths = { "exercise", "lastResult", "lastRatedResult" }) List findAllByExercise(Exercise exercise); diff --git a/src/main/java/de/tum/cit/aet/artemis/atlas/service/competency/StandardizedCompetencyService.java b/src/main/java/de/tum/cit/aet/artemis/atlas/service/competency/StandardizedCompetencyService.java index 47a4bd801e8e..55999666aea5 100644 --- a/src/main/java/de/tum/cit/aet/artemis/atlas/service/competency/StandardizedCompetencyService.java +++ b/src/main/java/de/tum/cit/aet/artemis/atlas/service/competency/StandardizedCompetencyService.java @@ -191,6 +191,7 @@ public void importStandardizedCompetencyCatalog(StandardizedCompetencyCatalogDTO */ public String exportStandardizedCompetencyCatalog() { List knowledgeAreas = getAllForTreeView(); + // TODO: we should avoid using findAll() here, as it might return a huge amount of data List sources = sourceRepository.findAll(); var catalog = StandardizedCompetencyCatalogDTO.of(knowledgeAreas, sources); diff --git a/src/main/java/de/tum/cit/aet/artemis/core/dto/CourseGroupsDTO.java b/src/main/java/de/tum/cit/aet/artemis/core/dto/CourseGroupsDTO.java new file mode 100644 index 000000000000..3aa5bce0b285 --- /dev/null +++ b/src/main/java/de/tum/cit/aet/artemis/core/dto/CourseGroupsDTO.java @@ -0,0 +1,7 @@ +package de.tum.cit.aet.artemis.core.dto; + +import com.fasterxml.jackson.annotation.JsonInclude; + +@JsonInclude(JsonInclude.Include.NON_EMPTY) +public record CourseGroupsDTO(String instructorGroupName, String editorGroupName, String teachingAssistantGroupName, String studentGroupName) { +} diff --git a/src/main/java/de/tum/cit/aet/artemis/core/repository/CourseRepository.java b/src/main/java/de/tum/cit/aet/artemis/core/repository/CourseRepository.java index b7b34537848d..8d9aa0c09df5 100644 --- a/src/main/java/de/tum/cit/aet/artemis/core/repository/CourseRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/core/repository/CourseRepository.java @@ -26,6 +26,7 @@ import de.tum.cit.aet.artemis.core.domain.Organization; import de.tum.cit.aet.artemis.core.domain.User; import de.tum.cit.aet.artemis.core.dto.CourseForArchiveDTO; +import de.tum.cit.aet.artemis.core.dto.CourseGroupsDTO; import de.tum.cit.aet.artemis.core.dto.StatisticsEntry; import de.tum.cit.aet.artemis.core.exception.EntityNotFoundException; import de.tum.cit.aet.artemis.core.repository.base.ArtemisJpaRepository; @@ -323,14 +324,6 @@ GROUP BY SUBSTRING(CAST(s.submissionDate AS string), 1, 10), p.student.login """) List findAllNotEndedCoursesByManagementGroupNames(@Param("now") ZonedDateTime now, @Param("userGroups") List userGroups); - @Query(""" - SELECT COUNT(DISTINCT ug.userId) - FROM Course c - JOIN UserGroup ug ON c.studentGroupName = ug.group - WHERE c.id = :courseId - """) - int countCourseStudents(@Param("courseId") long courseId); - /** * Counts the number of members of a course, i.e. users that are a member of the course's student, tutor, editor or instructor group. * Users that are part of multiple groups are NOT counted multiple times. @@ -569,4 +562,23 @@ SELECT COUNT(c) > 0 Set findInactiveCoursesForUserRolesWithNonNullSemester(@Param("isAdmin") boolean isAdmin, @Param("groups") Set groups, @Param("now") ZonedDateTime now); + @Query(""" + SELECT new de.tum.cit.aet.artemis.core.dto.CourseGroupsDTO( + c.instructorGroupName, + c.editorGroupName, + c.teachingAssistantGroupName, + c.studentGroupName + ) FROM Course c + """) + Set findAllCourseGroups(); + + @Query(""" + SELECT c + FROM Course c + WHERE c.teachingAssistantGroupName IN :userGroups + OR c.editorGroupName IN :userGroups + OR c.instructorGroupName IN :userGroups + OR :isAdmin = TRUE + """) + List findCoursesForAtLeastTutorWithGroups(@Param("userGroups") Set userGroups, @Param("isAdmin") boolean isAdmin); } diff --git a/src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java b/src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java index a286744dbe8a..81aa6a27f6b0 100644 --- a/src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java +++ b/src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java @@ -678,6 +678,7 @@ public List getAllCoursesForManagementOverview(boolean onlyActive) { var user = userRepository.getUserWithGroupsAndAuthorities(); boolean isAdmin = authCheckService.isAdmin(user); if (isAdmin && !onlyActive) { + // TODO: we should avoid using findAll() here, as it might return a huge amount of data return courseRepository.findAll(); } diff --git a/src/main/java/de/tum/cit/aet/artemis/core/web/AccountResource.java b/src/main/java/de/tum/cit/aet/artemis/core/web/AccountResource.java index 2a441e4cd035..8a525f751e89 100644 --- a/src/main/java/de/tum/cit/aet/artemis/core/web/AccountResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/core/web/AccountResource.java @@ -17,7 +17,6 @@ import org.apache.sshd.common.config.keys.AuthorizedKeyEntry; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Profile; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.DeleteMapping; @@ -58,9 +57,6 @@ public class AccountResource { public static final String ENTITY_NAME = "user"; - @Value("${jhipster.clientApp.name}") - private String applicationName; - private static final Logger log = LoggerFactory.getLogger(AccountResource.class); private final UserRepository userRepository; diff --git a/src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java b/src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java index 16b7dd554842..ba364d0c4fb5 100644 --- a/src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java @@ -21,7 +21,6 @@ import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; -import java.util.stream.Stream; import jakarta.validation.constraints.NotNull; @@ -99,6 +98,7 @@ import de.tum.cit.aet.artemis.core.security.annotations.EnforceAtLeastStudent; import de.tum.cit.aet.artemis.core.security.annotations.EnforceAtLeastTutor; import de.tum.cit.aet.artemis.core.security.annotations.enforceRoleInCourse.EnforceAtLeastEditorInCourse; +import de.tum.cit.aet.artemis.core.security.annotations.enforceRoleInCourse.EnforceAtLeastTutorInCourse; import de.tum.cit.aet.artemis.core.service.AuthorizationCheckService; import de.tum.cit.aet.artemis.core.service.CourseService; import de.tum.cit.aet.artemis.core.service.FilePathService; @@ -117,7 +117,6 @@ import de.tum.cit.aet.artemis.exercise.repository.TeamRepository; import de.tum.cit.aet.artemis.exercise.service.ExerciseService; import de.tum.cit.aet.artemis.exercise.service.SubmissionService; -import de.tum.cit.aet.artemis.lti.domain.OnlineCourseConfiguration; import de.tum.cit.aet.artemis.lti.service.OnlineCourseConfigurationService; import de.tum.cit.aet.artemis.programming.service.ci.CIUserManagementService; import de.tum.cit.aet.artemis.programming.service.vcs.VcsUserManagementService; @@ -255,16 +254,10 @@ public ResponseEntity updateCourse(@PathVariable Long courseId, @Request // this is important, otherwise someone could put himself into the instructor group of the updated course authCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.INSTRUCTOR, existingCourse, user); - Set existingGroupNames = new HashSet<>(List.of(existingCourse.getStudentGroupName(), existingCourse.getTeachingAssistantGroupName(), - existingCourse.getEditorGroupName(), existingCourse.getInstructorGroupName())); - Set newGroupNames = new HashSet<>(List.of(courseUpdate.getStudentGroupName(), courseUpdate.getTeachingAssistantGroupName(), courseUpdate.getEditorGroupName(), - courseUpdate.getInstructorGroupName())); - Set changedGroupNames = new HashSet<>(newGroupNames); - changedGroupNames.removeAll(existingGroupNames); - if (!authCheckService.isAdmin(user)) { // this means the user must be an instructor, who has NO Admin rights. // instructors are not allowed to change group names, because this would lead to security problems + final var changedGroupNames = getChangedGroupNames(courseUpdate, existingCourse); if (!changedGroupNames.isEmpty()) { throw new BadRequestAlertException("You are not allowed to change the group names of a course", Course.ENTITY_NAME, "groupNamesCannotChange", true); } @@ -367,48 +360,14 @@ else if (courseUpdate.getCourseIcon() == null && existingCourse.getCourseIcon() return ResponseEntity.ok(result); } - /** - * PUT courses/:courseId/online-course-configuration : Updates the onlineCourseConfiguration for the given course. - * - * @param courseId the id of the course to update - * @param onlineCourseConfiguration the online course configuration to update - * @return the ResponseEntity with status 200 (OK) and with body the updated online course configuration - */ - // TODO: move into LTIResource - @PutMapping("courses/{courseId}/online-course-configuration") - @EnforceAtLeastInstructor - @Profile(PROFILE_LTI) - public ResponseEntity updateOnlineCourseConfiguration(@PathVariable Long courseId, - @RequestBody OnlineCourseConfiguration onlineCourseConfiguration) { - log.debug("REST request to update the online course configuration for Course : {}", courseId); - - Course course = courseRepository.findByIdWithEagerOnlineCourseConfigurationElseThrow(courseId); - authCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.INSTRUCTOR, course, null); - - if (!course.isOnlineCourse()) { - throw new BadRequestAlertException("Course must be online course", Course.ENTITY_NAME, "courseMustBeOnline"); - } - - if (!course.getOnlineCourseConfiguration().getId().equals(onlineCourseConfiguration.getId())) { - throw new BadRequestAlertException("The onlineCourseConfigurationId does not match the id of the course's onlineCourseConfiguration", - OnlineCourseConfiguration.ENTITY_NAME, "idMismatch"); - } - - if (onlineCourseConfigurationService.isPresent()) { - onlineCourseConfigurationService.get().validateOnlineCourseConfiguration(onlineCourseConfiguration); - course.setOnlineCourseConfiguration(onlineCourseConfiguration); - try { - onlineCourseConfigurationService.get().addOnlineCourseConfigurationToLtiConfigurations(onlineCourseConfiguration); - } - catch (Exception ex) { - log.error("Failed to add online course configuration to LTI configurations", ex); - throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "Error when adding online course configuration to LTI configurations", ex); - } - } - - courseRepository.save(course); - - return ResponseEntity.ok(onlineCourseConfiguration); + private static Set getChangedGroupNames(Course courseUpdate, Course existingCourse) { + Set existingGroupNames = new HashSet<>(List.of(existingCourse.getStudentGroupName(), existingCourse.getTeachingAssistantGroupName(), + existingCourse.getEditorGroupName(), existingCourse.getInstructorGroupName())); + Set newGroupNames = new HashSet<>(List.of(courseUpdate.getStudentGroupName(), courseUpdate.getTeachingAssistantGroupName(), courseUpdate.getEditorGroupName(), + courseUpdate.getInstructorGroupName())); + Set changedGroupNames = new HashSet<>(newGroupNames); + changedGroupNames.removeAll(existingGroupNames); + return changedGroupNames; } /** @@ -479,17 +438,20 @@ public ResponseEntity> unenrollFromCourse(@PathVariable Long courseI @GetMapping("courses") @EnforceAtLeastTutor public ResponseEntity> getCourses(@RequestParam(defaultValue = "false") boolean onlyActive) { - log.debug("REST request to get all Courses the user has access to"); + log.debug("REST request to get all courses the user has access to"); User user = userRepository.getUserWithGroupsAndAuthorities(); - // TODO: we should avoid findAll() and instead try to filter this directly in the database, in case of admins, we should load batches of courses, e.g. per semester - List courses = courseRepository.findAll(); - Stream userCourses = courses.stream().filter(course -> user.getGroups().contains(course.getTeachingAssistantGroupName()) - || user.getGroups().contains(course.getInstructorGroupName()) || authCheckService.isAdmin(user)); + List courses = getCoursesForTutors(user, onlyActive); + return ResponseEntity.ok(courses); + } + + private List getCoursesForTutors(User user, boolean onlyActive) { + List userCourses = courseRepository.findCoursesForAtLeastTutorWithGroups(user.getGroups(), authCheckService.isAdmin(user)); if (onlyActive) { // only include courses that have NOT been finished - userCourses = userCourses.filter(course -> course.getEndDate() == null || course.getEndDate().isAfter(ZonedDateTime.now())); + final var now = ZonedDateTime.now(); + userCourses = userCourses.stream().filter(course -> course.getEndDate() == null || course.getEndDate().isAfter(now)).toList(); } - return ResponseEntity.ok(userCourses.toList()); + return userCourses; } /** @@ -536,8 +498,9 @@ public ResponseEntity> getCoursesWithQuizExercises() { @EnforceAtLeastTutor public ResponseEntity> getCoursesWithUserStats(@RequestParam(defaultValue = "false") boolean onlyActive) { log.debug("get courses with user stats, only active: {}", onlyActive); - // TODO: we should avoid using an endpoint in such cases and instead call a service method - List courses = getCourses(onlyActive).getBody(); + + User user = userRepository.getUserWithGroupsAndAuthorities(); + List courses = getCoursesForTutors(user, onlyActive); for (Course course : courses) { course.setNumberOfInstructors(userRepository.countUserInGroup(course.getInstructorGroupName())); course.setNumberOfTeachingAssistants(userRepository.countUserInGroup(course.getTeachingAssistantGroupName())); @@ -645,11 +608,11 @@ public ResponseEntity getCourseForDashboard(@PathVariable } courseService.fetchParticipationsWithSubmissionsAndResultsForCourses(List.of(course), user, true); - log.debug("courseService.fetchParticipationsWithSubmissionsAndResultsForCourses done"); + log.debug("courseService.fetchParticipationsWithSubmissionsAndResultsForCourses done in getCourseForDashboard"); courseService.fetchPlagiarismCasesForCourseExercises(course.getExercises(), user.getId()); - log.debug("courseService.fetchPlagiarismCasesForCourseExercises done"); + log.debug("courseService.fetchPlagiarismCasesForCourseExercises done in getCourseForDashboard"); GradingScale gradingScale = gradingScaleRepository.findByCourseId(course.getId()).orElse(null); - log.debug("gradingScaleRepository.findByCourseId done"); + log.debug("gradingScaleRepository.findByCourseId done in getCourseForDashboard"); CourseForDashboardDTO courseForDashboardDTO = courseScoreCalculationService.getScoresAndParticipationResults(course, gradingScale, user.getId()); logDuration(List.of(course), user, timeNanoStart, "courses/" + courseId + "/for-dashboard (single course)"); return ResponseEntity.ok(courseForDashboardDTO); @@ -748,16 +711,15 @@ public ResponseEntity> getCoursesForNotifications() { * @return data about a course including all exercises, plus some data for the tutor as tutor status for assessment */ @GetMapping("courses/{courseId}/for-assessment-dashboard") - @EnforceAtLeastTutor + @EnforceAtLeastTutorInCourse public ResponseEntity getCourseForAssessmentDashboard(@PathVariable long courseId) { log.debug("REST request /courses/{courseId}/for-assessment-dashboard"); - // TODO: use ...ElseThrow below in case the course cannot be found - Course course = courseRepository.findWithEagerExercisesById(courseId); - User user = userRepository.getUserWithGroupsAndAuthorities(); - authCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.TEACHING_ASSISTANT, course, user); + Course course = courseRepository.findByIdWithEagerExercisesElseThrow(courseId); Set interestingExercises = courseRepository.filterInterestingExercisesForAssessmentDashboards(course.getExercises()); course.setExercises(interestingExercises); + + User user = userRepository.getUser(); List tutorParticipations = tutorParticipationRepository.findAllByAssessedExercise_Course_IdAndTutor_Id(course.getId(), user.getId()); assessmentDashboardService.generateStatisticsForExercisesForAssessmentDashboard(course.getExercises(), tutorParticipations, false); return ResponseEntity.ok(course); @@ -790,7 +752,7 @@ public ResponseEntity getStatsForAssessmentDashboard(@Path @GetMapping("courses/{courseId}") @EnforceAtLeastStudent public ResponseEntity getCourse(@PathVariable Long courseId) { - log.debug("REST request to get Course : {}", courseId); + log.debug("REST request to get course {} for students", courseId); Course course = courseRepository.findByIdElseThrow(courseId); User user = userRepository.getUserWithGroupsAndAuthorities(); @@ -822,7 +784,7 @@ else if (authCheckService.isAtLeastTeachingAssistantInCourse(course, user)) { @GetMapping("courses/{courseId}/with-exercises") @EnforceAtLeastTutor public ResponseEntity getCourseWithExercises(@PathVariable Long courseId) { - log.debug("REST request to get Course : {}", courseId); + log.debug("REST request to get course {} for tutors", courseId); Course course = courseRepository.findWithEagerExercisesById(courseId); authCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.TEACHING_ASSISTANT, course, null); return ResponseEntity.ok(course); @@ -1069,20 +1031,9 @@ public ResponseEntity> searchUsersInCourse(@PathVariable if (loginOrName.length() < 3 && requestedRoles.contains(Role.STUDENT)) { throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Query param 'loginOrName' must be three characters or longer if you search for students."); } - var groups = new HashSet(); - if (requestedRoles.contains(Role.STUDENT)) { - groups.add(course.getStudentGroupName()); - } - if (requestedRoles.contains(Role.TEACHING_ASSISTANT)) { - groups.add(course.getTeachingAssistantGroupName()); - // searching for tutors also searches for editors - groups.add(course.getEditorGroupName()); - } - if (requestedRoles.contains(Role.INSTRUCTOR)) { - groups.add(course.getInstructorGroupName()); - } + final var relevantCourseGroupNames = getRelevantCourseGroupNames(requestedRoles, course); User searchingUser = userRepository.getUser(); - var originalPage = userRepository.searchAllWithGroupsByLoginOrNameInGroupsNotUserId(PageRequest.of(0, 25), loginOrName, groups, searchingUser.getId()); + var originalPage = userRepository.searchAllWithGroupsByLoginOrNameInGroupsNotUserId(PageRequest.of(0, 25), loginOrName, relevantCourseGroupNames, searchingUser.getId()); var resultDTOs = new ArrayList(); for (var user : originalPage) { @@ -1097,6 +1048,22 @@ public ResponseEntity> searchUsersInCourse(@PathVariable return new ResponseEntity<>(dtoPage.getContent(), headers, HttpStatus.OK); } + private static HashSet getRelevantCourseGroupNames(Set requestedRoles, Course course) { + var groups = new HashSet(); + if (requestedRoles.contains(Role.STUDENT)) { + groups.add(course.getStudentGroupName()); + } + if (requestedRoles.contains(Role.TEACHING_ASSISTANT)) { + groups.add(course.getTeachingAssistantGroupName()); + // searching for tutors also searches for editors + groups.add(course.getEditorGroupName()); + } + if (requestedRoles.contains(Role.INSTRUCTOR)) { + groups.add(course.getInstructorGroupName()); + } + return groups; + } + /** * GET /courses/:courseId/tutors : Returns all users that belong to the tutor group of the course * diff --git a/src/main/java/de/tum/cit/aet/artemis/core/web/FileResource.java b/src/main/java/de/tum/cit/aet/artemis/core/web/FileResource.java index 0e90597a25bd..ac1e03f866da 100644 --- a/src/main/java/de/tum/cit/aet/artemis/core/web/FileResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/core/web/FileResource.java @@ -416,7 +416,7 @@ public ResponseEntity getExamUserImage(@PathVariable Long examUserId) { @GetMapping("files/attachments/lecture/{lectureId}/{filename}") @EnforceAtLeastStudent public ResponseEntity getLectureAttachment(@PathVariable Long lectureId, @PathVariable String filename) { - log.debug("REST request to get file : {}", filename); + log.debug("REST request to get lecture attachment : {}", filename); String fileNameWithoutSpaces = filename.replaceAll(" ", "_"); sanitizeFilenameElseThrow(fileNameWithoutSpaces); @@ -434,24 +434,6 @@ public ResponseEntity getLectureAttachment(@PathVariable Long lectureId, return buildFileResponse(getActualPathFromPublicPathString(attachment.getLink()), false); } - /** - * GET /files/courses/{courseId}/attachments/{attachmentId} : Returns the file associated with the - * given attachment ID as a downloadable resource - * - * @param courseId The ID of the course that the Attachment belongs to - * @param attachmentId the ID of the attachment to retrieve - * @return ResponseEntity containing the file as a resource - */ - @GetMapping("files/courses/{courseId}/attachments/{attachmentId}") - @EnforceAtLeastEditorInCourse - public ResponseEntity getAttachmentFile(@PathVariable Long courseId, @PathVariable Long attachmentId) { - Attachment attachment = attachmentRepository.findByIdElseThrow(attachmentId); - Course course = courseRepository.findByIdElseThrow(courseId); - checkAttachmentExistsInCourseOrThrow(course, attachment); - - return buildFileResponse(getActualPathFromPublicPathString(attachment.getLink()), false); - } - /** * GET /files/attachments/lecture/{lectureId}/merge-pdf : Get the lecture units * PDF attachments merged @@ -494,8 +476,9 @@ public ResponseEntity getLecturePdfAttachmentsMerged(@PathVariable Long */ @GetMapping("files/attachments/attachment-unit/{attachmentUnitId}/*") @EnforceAtLeastStudent + // TODO: this method is kind of redundant to the method getAttachmentFile below, double check if both are actually needed public ResponseEntity getAttachmentUnitAttachment(@PathVariable Long attachmentUnitId) { - log.debug("REST request to get file for attachment unit : {}", attachmentUnitId); + log.debug("REST request to get the file for attachment unit {} for students", attachmentUnitId); AttachmentUnit attachmentUnit = attachmentUnitRepository.findByIdElseThrow(attachmentUnitId); // get the course for a lecture's attachment unit @@ -509,7 +492,7 @@ public ResponseEntity getAttachmentUnitAttachment(@PathVariable Long att } /** - * GET files/courses/{courseId}/attachment-units/{attachmenUnitId} : Returns the file associated with the + * GET files/courses/{courseId}/attachment-units/{attachmentUnitId} : Returns the file associated with the * given attachmentUnit ID as a downloadable resource * * @param courseId The ID of the course that the Attachment belongs to @@ -519,7 +502,7 @@ public ResponseEntity getAttachmentUnitAttachment(@PathVariable Long att @GetMapping("files/courses/{courseId}/attachment-units/{attachmentUnitId}") @EnforceAtLeastEditorInCourse public ResponseEntity getAttachmentUnitFile(@PathVariable Long courseId, @PathVariable Long attachmentUnitId) { - log.debug("REST request to get file for attachment unit : {}", attachmentUnitId); + log.debug("REST request to get the file for attachment unit {} for editors", attachmentUnitId); AttachmentUnit attachmentUnit = attachmentUnitRepository.findByIdElseThrow(attachmentUnitId); Course course = courseRepository.findByIdElseThrow(courseId); Attachment attachment = attachmentUnit.getAttachment(); @@ -528,6 +511,25 @@ public ResponseEntity getAttachmentUnitFile(@PathVariable Long courseId, return buildFileResponse(getActualPathFromPublicPathString(attachment.getLink()), false); } + /** + * GET /files/courses/{courseId}/attachments/{attachmentId} : Returns the file associated with the + * given attachment ID as a downloadable resource + * + * @param courseId The ID of the course that the Attachment belongs to + * @param attachmentId the ID of the attachment to retrieve + * @return ResponseEntity containing the file as a resource + */ + @GetMapping("files/courses/{courseId}/attachments/{attachmentId}") + @EnforceAtLeastEditorInCourse + public ResponseEntity getAttachmentFile(@PathVariable Long courseId, @PathVariable Long attachmentId) { + log.debug("REST request to get attachment file : {}", attachmentId); + Attachment attachment = attachmentRepository.findByIdElseThrow(attachmentId); + Course course = courseRepository.findByIdElseThrow(courseId); + checkAttachmentExistsInCourseOrThrow(course, attachment); + + return buildFileResponse(getActualPathFromPublicPathString(attachment.getLink()), false); + } + /** * GET files/attachments/attachment-unit/{attachmentUnitId}/slide/{slideNumber} : Get the lecture unit attachment slide by slide number * diff --git a/src/main/java/de/tum/cit/aet/artemis/core/web/admin/AdminCourseResource.java b/src/main/java/de/tum/cit/aet/artemis/core/web/admin/AdminCourseResource.java index 6c26cab4798f..2ac600477861 100644 --- a/src/main/java/de/tum/cit/aet/artemis/core/web/admin/AdminCourseResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/core/web/admin/AdminCourseResource.java @@ -6,7 +6,7 @@ import java.net.URISyntaxException; import java.nio.file.Path; import java.util.Arrays; -import java.util.LinkedHashSet; +import java.util.HashSet; import java.util.List; import java.util.Optional; import java.util.Set; @@ -35,6 +35,7 @@ import de.tum.cit.aet.artemis.core.domain.Course; import de.tum.cit.aet.artemis.core.domain.User; import de.tum.cit.aet.artemis.core.dto.CourseDeletionSummaryDTO; +import de.tum.cit.aet.artemis.core.dto.CourseGroupsDTO; import de.tum.cit.aet.artemis.core.exception.BadRequestAlertException; import de.tum.cit.aet.artemis.core.repository.CourseRepository; import de.tum.cit.aet.artemis.core.repository.UserRepository; @@ -94,14 +95,14 @@ public AdminCourseResource(UserRepository userRepository, CourseService courseSe @GetMapping("courses/groups") public ResponseEntity> getAllGroupsForAllCourses() { log.debug("REST request to get all Groups for all Courses"); - List courses = courseRepository.findAll(); - Set groups = new LinkedHashSet<>(); - for (Course course : courses) { - groups.add(course.getInstructorGroupName()); - groups.add(course.getEditorGroupName()); - groups.add(course.getTeachingAssistantGroupName()); - groups.add(course.getStudentGroupName()); - } + Set courseGroups = courseRepository.findAllCourseGroups(); + Set groups = new HashSet<>(); + courseGroups.forEach(courseGroup -> { + groups.add(courseGroup.instructorGroupName()); + groups.add(courseGroup.editorGroupName()); + groups.add(courseGroup.teachingAssistantGroupName()); + groups.add(courseGroup.studentGroupName()); + }); return ResponseEntity.ok().body(groups); } diff --git a/src/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.java b/src/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.java index 3681b3a94221..b617ae44948f 100644 --- a/src/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/exercise/repository/StudentParticipationRepository.java @@ -417,7 +417,7 @@ default List findByExerciseIdWithManualResultAndFeedbacksA LEFT JOIN FETCH p.submissions WHERE p.exercise.id = :exerciseId AND p.student.id = :studentId - """) + """) List findByExerciseIdAndStudentIdWithEagerResultsAndSubmissions(@Param("exerciseId") long exerciseId, @Param("studentId") long studentId); @Query(""" @@ -455,12 +455,11 @@ default List findByExerciseIdWithManualResultAndFeedbacksA """) List findByExerciseIdAndTeamIdWithEagerResultsAndLegalSubmissionsAndTeamStudents(@Param("exerciseId") long exerciseId, @Param("teamId") long teamId); + // NOTE: we should not fetch too elements here so we leave out feedback and test cases, otherwise the query will be very slow @Query(""" SELECT DISTINCT p FROM StudentParticipation p LEFT JOIN FETCH p.results r - LEFT JOIN FETCH r.feedbacks f - LEFT JOIN FETCH f.testCase LEFT JOIN FETCH r.submission s WHERE p.exercise.id = :exerciseId AND p.student.id = :studentId @@ -487,13 +486,12 @@ Optional findByExerciseIdAndStudentIdAndTestRunWithLatestR * @param exerciseId the exercise id the participations should belong to * @return a list of participations including their submitted submissions that do not have a manual result */ + // NOTE: we should not fetch too elements here so we leave out feedback and test cases, otherwise the query will be very slow @Query(""" SELECT DISTINCT p FROM StudentParticipation p LEFT JOIN FETCH p.submissions submission LEFT JOIN FETCH submission.results result - LEFT JOIN FETCH result.feedbacks feedbacks - LEFT JOIN FETCH feedbacks.testCase LEFT JOIN FETCH result.assessor WHERE p.exercise.id = :exerciseId AND p.testRun = FALSE @@ -503,7 +501,8 @@ SELECT COUNT(r2) WHERE r2.assessor IS NOT NULL AND (r2.rated IS NULL OR r2.rated = FALSE) AND r2.submission = submission - ) AND :correctionRound = ( + ) + AND :correctionRound = ( SELECT COUNT(r) FROM Result r WHERE r.assessor IS NOT NULL @@ -514,14 +513,16 @@ AND r.assessmentType IN ( de.tum.cit.aet.artemis.assessment.domain.AssessmentType.MANUAL, de.tum.cit.aet.artemis.assessment.domain.AssessmentType.SEMI_AUTOMATIC ) AND (p.exercise.dueDate IS NULL OR r.submission.submissionDate <= p.exercise.dueDate) - ) AND :correctionRound = ( + ) + AND :correctionRound = ( SELECT COUNT(prs) FROM p.results prs WHERE prs.assessmentType IN ( de.tum.cit.aet.artemis.assessment.domain.AssessmentType.MANUAL, de.tum.cit.aet.artemis.assessment.domain.AssessmentType.SEMI_AUTOMATIC ) - ) AND submission.submitted = TRUE + ) + AND submission.submitted = TRUE AND submission.id = (SELECT MAX(s.id) FROM p.submissions s) """) List findByExerciseIdWithLatestSubmissionWithoutManualResultsAndIgnoreTestRunParticipation(@Param("exerciseId") long exerciseId, @@ -529,13 +530,12 @@ List findByExerciseIdWithLatestSubmissionWithoutManualResu Set findDistinctAllByExerciseIdInAndStudentId(Set exerciseIds, Long studentId); + // NOTE: we should not fetch too elements here so we leave out feedback and test cases, otherwise the query will be very slow @Query(""" SELECT DISTINCT p FROM Participation p LEFT JOIN FETCH p.submissions s LEFT JOIN FETCH s.results r - LEFT JOIN FETCH r.feedbacks f - LEFT JOIN FETCH f.testCase WHERE p.exercise.id = :exerciseId AND (p.individualDueDate IS NULL OR p.individualDueDate <= :now) AND p.testRun = FALSE @@ -1016,7 +1016,7 @@ default List findByStudentExamWithEagerSubmissions(Student * Get a mapping of participation ids to the number of submission for each participation. * * @param exerciseId the id of the exercise for which to consider participations - * @return the number of submissions per participation in the given exercise + * @return a map of submissions per participation in the given exercise */ default Map countSubmissionsPerParticipationByExerciseIdAsMap(long exerciseId) { return convertListOfCountsIntoMap(countSubmissionsPerParticipationByExerciseId(exerciseId)); @@ -1027,7 +1027,7 @@ default Map countSubmissionsPerParticipationByExerciseIdAsMap(lon * * @param courseId the id of the course for which to consider participations * @param teamShortName the short name of the team for which to consider participations - * @return the number of submissions per participation in the given course for the team + * @return a map of submissions per participation in the given course for the team */ default Map countLegalSubmissionsPerParticipationByCourseIdAndTeamShortNameAsMap(long courseId, String teamShortName) { return convertListOfCountsIntoMap(countLegalSubmissionsPerParticipationByCourseIdAndTeamShortName(courseId, teamShortName)); @@ -1311,6 +1311,7 @@ SELECT MAX(pr.id) * @param exerciseId The ID of the exercise for which the maximum feedback count is to be retrieved. * @return The maximum count of feedback occurrences for the given exercise. */ + // TODO: move this query to a more appropriate repository, either feedbackRepository or exerciseRepository @Query(""" SELECT MAX(feedbackCounts.feedbackCount) FROM ( diff --git a/src/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.java b/src/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.java index 2669e856337f..006d52ac1eb7 100644 --- a/src/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.java +++ b/src/main/java/de/tum/cit/aet/artemis/exercise/service/SubmissionService.java @@ -203,10 +203,13 @@ public List getAllSubmissionsAssessedByTutorForCorrect } protected List getAssessableSubmissions(Exercise exercise, boolean examMode, int correctionRound) { + // TODO: it really does not make sense to fetch these submissions with all related data from the database just to select one submission afterwards + // it would be better to fetch them with minimal related data (so we can select one) and then afterwards fetch the selected one with all related data + final List participations; if (examMode) { // Get all participations of submissions that are submitted and do not already have a manual result or belong to test run submissions. - // No manual result means that no user has started an assessment for the corresponding submission yet. + // No manual result means that no tutor has started an assessment for the corresponding submission yet. participations = studentParticipationRepository.findByExerciseIdWithLatestSubmissionWithoutManualResultsAndIgnoreTestRunParticipation(exercise.getId(), correctionRound); } @@ -218,17 +221,21 @@ protected List getAssessableSubmissions(Exercise exercise, boolean e ZonedDateTime.now()); } - List submissionsWithoutResult = participations.stream().map(Participation::findLatestLegalOrIllegalSubmission).filter(Optional::isPresent).map(Optional::get) - .toList(); + // TODO: we could move the ILLEGAL check into the database + var submissionsWithoutResult = participations.stream().map(Participation::findLatestLegalOrIllegalSubmission).filter(Optional::isPresent).map(Optional::get).toList(); if (correctionRound > 0) { // remove submission if user already assessed first correction round // if disabled, please switch tutorAssessUnique within the tests - submissionsWithoutResult = submissionsWithoutResult.stream() - .filter(submission -> !submission.getResultForCorrectionRound(correctionRound - 1).getAssessor().equals(userRepository.getUser())).toList(); + // TODO: we could move this check into the database call of the if clause above (examMode == true) to avoid fetching all results and assessors + final var user = userRepository.getUser(); + submissionsWithoutResult = submissionsWithoutResult.stream().filter(submission -> { + final var resultForCorrectionRound = submission.getResultForCorrectionRound(correctionRound - 1); + return resultForCorrectionRound != null && !resultForCorrectionRound.getAssessor().equals(user); + }).toList(); } - if (exercise.getDueDate() != null) { + if (!examMode && exercise.getDueDate() != null) { submissionsWithoutResult = selectOnlySubmissionsBeforeDueDate(submissionsWithoutResult); } diff --git a/src/main/java/de/tum/cit/aet/artemis/fileupload/service/FileUploadSubmissionService.java b/src/main/java/de/tum/cit/aet/artemis/fileupload/service/FileUploadSubmissionService.java index ce3cc4b157d6..80d732be3981 100644 --- a/src/main/java/de/tum/cit/aet/artemis/fileupload/service/FileUploadSubmissionService.java +++ b/src/main/java/de/tum/cit/aet/artemis/fileupload/service/FileUploadSubmissionService.java @@ -255,9 +255,11 @@ public FileUploadSubmission lockAndGetFileUploadSubmission(Long submissionId, Fi * @return a locked file upload submission that needs an assessment */ public FileUploadSubmission lockAndGetFileUploadSubmissionWithoutResult(FileUploadExercise fileUploadExercise, boolean ignoreTestRunParticipations, int correctionRound) { - FileUploadSubmission fileUploadSubmission = getRandomFileUploadSubmissionEligibleForNewAssessment(fileUploadExercise, ignoreTestRunParticipations, correctionRound) + var submission = getRandomFileUploadSubmissionEligibleForNewAssessment(fileUploadExercise, ignoreTestRunParticipations, correctionRound) .orElseThrow(() -> new EntityNotFoundException("File upload submission for exercise " + fileUploadExercise.getId() + " could not be found")); - lockSubmission(fileUploadSubmission, correctionRound); - return fileUploadSubmission; + // NOTE: we load the feedback for the submission eagerly to avoid org.hibernate.LazyInitializationException + submission = fileUploadSubmissionRepository.findByIdWithEagerResultAndFeedbackAndAssessorAndAssessmentNoteAndParticipationResultsElseThrow(submission.getId()); + lockSubmission(submission, correctionRound); + return submission; } } diff --git a/src/main/java/de/tum/cit/aet/artemis/fileupload/web/FileUploadSubmissionResource.java b/src/main/java/de/tum/cit/aet/artemis/fileupload/web/FileUploadSubmissionResource.java index 0d7c4a23280f..d036f978e157 100644 --- a/src/main/java/de/tum/cit/aet/artemis/fileupload/web/FileUploadSubmissionResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/fileupload/web/FileUploadSubmissionResource.java @@ -6,7 +6,6 @@ import java.util.Collections; import java.util.List; import java.util.Optional; -import java.util.Set; import jakarta.validation.constraints.NotNull; @@ -26,7 +25,6 @@ import org.springframework.web.multipart.MultipartFile; import org.springframework.web.server.ResponseStatusException; -import de.tum.cit.aet.artemis.assessment.domain.GradingCriterion; import de.tum.cit.aet.artemis.assessment.domain.Result; import de.tum.cit.aet.artemis.assessment.repository.GradingCriterionRepository; import de.tum.cit.aet.artemis.assessment.service.ResultService; @@ -256,18 +254,18 @@ public ResponseEntity getFileUploadSubmissionWithoutAssess if (!(fileUploadExercise instanceof FileUploadExercise)) { throw new BadRequestAlertException("The requested exercise was not found.", "exerciseId", "400"); } - Set gradingCriteria = gradingCriterionRepository.findByExerciseIdWithEagerGradingCriteria(exerciseId); - fileUploadExercise.setGradingCriteria(gradingCriteria); - final User user = userRepository.getUserWithGroupsAndAuthorities(); - + final var user = userRepository.getUserWithGroupsAndAuthorities(); authCheckService.checkHasAtLeastRoleForExerciseElseThrow(Role.TEACHING_ASSISTANT, fileUploadExercise, user); // Check if tutors can start assessing the students submission - this.fileUploadSubmissionService.checkIfExerciseDueDateIsReached(fileUploadExercise); + fileUploadSubmissionService.checkIfExerciseDueDateIsReached(fileUploadExercise); // Check if the limit of simultaneously locked submissions has been reached fileUploadSubmissionService.checkSubmissionLockLimit(fileUploadExercise.getCourseViaExerciseGroupOrCourseMember().getId()); + final var gradingCriteria = gradingCriterionRepository.findByExerciseIdWithEagerGradingCriteria(exerciseId); + fileUploadExercise.setGradingCriteria(gradingCriteria); + final FileUploadSubmission submission; if (lockSubmission) { submission = fileUploadSubmissionService.lockAndGetFileUploadSubmissionWithoutResult((FileUploadExercise) fileUploadExercise, fileUploadExercise.isExamExercise(), @@ -284,7 +282,7 @@ public ResponseEntity getFileUploadSubmissionWithoutAssess final StudentParticipation studentParticipation = (StudentParticipation) submission.getParticipation(); studentParticipation.setExercise(fileUploadExercise); submission.getParticipation().getExercise().setGradingCriteria(gradingCriteria); - this.fileUploadSubmissionService.hideDetails(submission, user); + fileUploadSubmissionService.hideDetails(submission, user); } return ResponseEntity.ok(submission); diff --git a/src/main/java/de/tum/cit/aet/artemis/lti/web/LtiResource.java b/src/main/java/de/tum/cit/aet/artemis/lti/web/LtiResource.java index 1dbfede94a83..cb27da1cf735 100644 --- a/src/main/java/de/tum/cit/aet/artemis/lti/web/LtiResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/lti/web/LtiResource.java @@ -18,9 +18,12 @@ import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.PutMapping; +import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.server.ResponseStatusException; import org.springframework.web.servlet.support.ServletUriComponentsBuilder; import com.fasterxml.jackson.databind.ObjectMapper; @@ -34,8 +37,10 @@ import de.tum.cit.aet.artemis.core.security.annotations.EnforceAtLeastInstructor; import de.tum.cit.aet.artemis.core.service.AuthorizationCheckService; import de.tum.cit.aet.artemis.lti.domain.LtiPlatformConfiguration; +import de.tum.cit.aet.artemis.lti.domain.OnlineCourseConfiguration; import de.tum.cit.aet.artemis.lti.repository.LtiPlatformConfigurationRepository; import de.tum.cit.aet.artemis.lti.service.LtiDeepLinkingService; +import de.tum.cit.aet.artemis.lti.service.OnlineCourseConfigurationService; import tech.jhipster.web.util.PaginationUtil; /** @@ -50,6 +55,8 @@ public class LtiResource { private final LtiDeepLinkingService ltiDeepLinkingService; + private final OnlineCourseConfigurationService onlineCourseConfigurationService; + private final CourseRepository courseRepository; private final AuthorizationCheckService authCheckService; @@ -64,13 +71,55 @@ public class LtiResource { * @param ltiDeepLinkingService Service for LTI deep linking. */ public LtiResource(CourseRepository courseRepository, AuthorizationCheckService authCheckService, LtiDeepLinkingService ltiDeepLinkingService, - LtiPlatformConfigurationRepository ltiPlatformConfigurationRepository) { + OnlineCourseConfigurationService onlineCourseConfigurationService, LtiPlatformConfigurationRepository ltiPlatformConfigurationRepository) { this.courseRepository = courseRepository; this.authCheckService = authCheckService; this.ltiDeepLinkingService = ltiDeepLinkingService; + this.onlineCourseConfigurationService = onlineCourseConfigurationService; this.ltiPlatformConfigurationRepository = ltiPlatformConfigurationRepository; } + /** + * PUT courses/:courseId/online-course-configuration : Updates the onlineCourseConfiguration for the given course. + * + * @param courseId the id of the course to update + * @param onlineCourseConfiguration the online course configuration to update + * @return the ResponseEntity with status 200 (OK) and with body the updated online course configuration + */ + @PutMapping("courses/{courseId}/online-course-configuration") + @EnforceAtLeastInstructor + @Profile(PROFILE_LTI) + public ResponseEntity updateOnlineCourseConfiguration(@PathVariable Long courseId, + @RequestBody OnlineCourseConfiguration onlineCourseConfiguration) { + log.debug("REST request to update the online course configuration for Course : {}", courseId); + + Course course = courseRepository.findByIdWithEagerOnlineCourseConfigurationElseThrow(courseId); + authCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.INSTRUCTOR, course, null); + + if (!course.isOnlineCourse()) { + throw new BadRequestAlertException("Course must be online course", Course.ENTITY_NAME, "courseMustBeOnline"); + } + + if (!course.getOnlineCourseConfiguration().getId().equals(onlineCourseConfiguration.getId())) { + throw new BadRequestAlertException("The onlineCourseConfigurationId does not match the id of the course's onlineCourseConfiguration", + OnlineCourseConfiguration.ENTITY_NAME, "idMismatch"); + } + + onlineCourseConfigurationService.validateOnlineCourseConfiguration(onlineCourseConfiguration); + course.setOnlineCourseConfiguration(onlineCourseConfiguration); + try { + onlineCourseConfigurationService.addOnlineCourseConfigurationToLtiConfigurations(onlineCourseConfiguration); + } + catch (Exception ex) { + log.error("Failed to add online course configuration to LTI configurations", ex); + throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "Error when adding online course configuration to LTI configurations", ex); + } + + courseRepository.save(course); + + return ResponseEntity.ok(onlineCourseConfiguration); + } + /** * Handles the HTTP POST request for LTI 1.3 Deep Linking. This endpoint is used for deep linking of LTI links * for exercises within a course. The method populates content items with the provided course and exercise identifiers, diff --git a/src/main/java/de/tum/cit/aet/artemis/modeling/repository/ModelingSubmissionRepository.java b/src/main/java/de/tum/cit/aet/artemis/modeling/repository/ModelingSubmissionRepository.java index 07eb0e37c28e..70f9b42ba4f7 100644 --- a/src/main/java/de/tum/cit/aet/artemis/modeling/repository/ModelingSubmissionRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/modeling/repository/ModelingSubmissionRepository.java @@ -39,7 +39,7 @@ public interface ModelingSubmissionRepository extends ArtemisJpaRepository findWithResultsFeedbacksAssessorAssessmentNoteAndParticipationResultsById(Long submissionId); @Query(""" diff --git a/src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingSubmissionService.java b/src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingSubmissionService.java index c8b4285de3b9..4172456a0a81 100644 --- a/src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingSubmissionService.java +++ b/src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingSubmissionService.java @@ -90,18 +90,17 @@ public ModelingSubmissionService(ModelingSubmissionRepository modelingSubmission * @return the locked modeling submission */ public ModelingSubmission lockAndGetModelingSubmission(Long submissionId, ModelingExercise modelingExercise, int correctionRound) { - ModelingSubmission modelingSubmission = modelingSubmissionRepository - .findByIdWithEagerResultAndFeedbackAndAssessorAndAssessmentNoteAndParticipationResultsElseThrow(submissionId); + var submission = modelingSubmissionRepository.findByIdWithEagerResultAndFeedbackAndAssessorAndAssessmentNoteAndParticipationResultsElseThrow(submissionId); - if (modelingSubmission.getLatestResult() == null || modelingSubmission.getLatestResult().getAssessor() == null) { + if (submission.getLatestResult() == null || submission.getLatestResult().getAssessor() == null) { checkSubmissionLockLimit(modelingExercise.getCourseViaExerciseGroupOrCourseMember().getId()); if (compassService.isSupported(modelingExercise) && correctionRound == 0L) { - modelingSubmission = assignResultWithFeedbackSuggestionsToSubmission(modelingSubmission, modelingExercise); + submission = assignResultWithFeedbackSuggestionsToSubmission(submission, modelingExercise); } } - lockSubmission(modelingSubmission, correctionRound); - return modelingSubmission; + lockSubmission(submission, correctionRound); + return submission; } /** @@ -196,27 +195,18 @@ public Optional findRandomSubmissionWithoutExistingAssessmen return Optional.empty(); } - ModelingSubmission modelingSubmission = (ModelingSubmission) submissionWithoutResult.get(); + // NOTE: we load the feedback for the submission eagerly to avoid org.hibernate.LazyInitializationException + var submissionId = submissionWithoutResult.get().getId(); + var submission = modelingSubmissionRepository.findByIdWithEagerResultAndFeedbackAndAssessorAndAssessmentNoteAndParticipationResultsElseThrow(submissionId); if (lockSubmission) { if (compassService.isSupported(modelingExercise) && correctionRound == 0L) { - modelingSubmission = assignResultWithFeedbackSuggestionsToSubmission(modelingSubmission, modelingExercise); - setNumberOfAffectedSubmissionsPerElement(modelingSubmission); + submission = assignResultWithFeedbackSuggestionsToSubmission(submission, modelingExercise); + setNumberOfAffectedSubmissionsPerElement(submission); } - lockSubmission(modelingSubmission, correctionRound); + lockSubmission(submission, correctionRound); } - return Optional.of(modelingSubmission); - } - - /** - * Soft lock the submission to prevent other tutors from receiving and assessing it. We remove the model from the models waiting for assessment in Compass to prevent other - * tutors from retrieving it in the first place. Additionally, we set the assessor and save the result to soft lock the assessment in the client, i.e. the client will not allow - * tutors to assess a model when an assessor is already assigned. If no result exists for this submission we create one first. - * - * @param modelingSubmission the submission to lock - */ - private void lockSubmission(ModelingSubmission modelingSubmission, int correctionRound) { - super.lockSubmission(modelingSubmission, correctionRound); + return Optional.of(submission); } /** diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/service/hestia/ProgrammingExerciseTaskService.java b/src/main/java/de/tum/cit/aet/artemis/programming/service/hestia/ProgrammingExerciseTaskService.java index 1684cf52c018..0b1a14be8646 100644 --- a/src/main/java/de/tum/cit/aet/artemis/programming/service/hestia/ProgrammingExerciseTaskService.java +++ b/src/main/java/de/tum/cit/aet/artemis/programming/service/hestia/ProgrammingExerciseTaskService.java @@ -350,7 +350,7 @@ private String convertTestNameToTestIdReplacement(String testName, Set getProgrammingSubmissionWithoutAsse if (submission != null) { if (lockSubmission) { - Result lockedResult = programmingSubmissionService.lockSubmission(submission, correctionRound); - submission = (ProgrammingSubmission) lockedResult.getSubmission(); + // NOTE: we explicitly load the feedback for the submission eagerly to avoid org.hibernate.LazyInitializationException + submission = programmingSubmissionService.lockAndGetProgrammingSubmission(submission.getId(), correctionRound); } submission.getParticipation().setExercise(programmingExercise); programmingSubmissionService.hideDetails(submission, user); diff --git a/src/main/java/de/tum/cit/aet/artemis/text/repository/TextSubmissionRepository.java b/src/main/java/de/tum/cit/aet/artemis/text/repository/TextSubmissionRepository.java index 9929c1b5f9db..604964966874 100644 --- a/src/main/java/de/tum/cit/aet/artemis/text/repository/TextSubmissionRepository.java +++ b/src/main/java/de/tum/cit/aet/artemis/text/repository/TextSubmissionRepository.java @@ -33,8 +33,8 @@ public interface TextSubmissionRepository extends ArtemisJpaRepository findWithEagerResultsById(long submissionId); + @EntityGraph(type = LOAD, attributePaths = { "results.assessor" }) + Optional findWithEagerResultsAssessorById(long submissionId); /** * @param submissionId the submission id we are interested in @@ -43,7 +43,7 @@ public interface TextSubmissionRepository extends ArtemisJpaRepository findWithEagerResultsAndFeedbackAndTextBlocksById(long submissionId); - @EntityGraph(type = LOAD, attributePaths = { "results", "results.assessor", "blocks", "results.feedbacks" }) + @EntityGraph(type = LOAD, attributePaths = { "results.assessor", "blocks", "results.feedbacks" }) Optional findWithEagerResultAndTextBlocksAndFeedbackByResults_Id(long resultId); @NotNull diff --git a/src/main/java/de/tum/cit/aet/artemis/text/service/TextSubmissionService.java b/src/main/java/de/tum/cit/aet/artemis/text/service/TextSubmissionService.java index a7f5ff325f1a..2df4ae21c2ab 100644 --- a/src/main/java/de/tum/cit/aet/artemis/text/service/TextSubmissionService.java +++ b/src/main/java/de/tum/cit/aet/artemis/text/service/TextSubmissionService.java @@ -155,11 +155,15 @@ public Optional getRandomTextSubmissionEligibleForNewAssessment( /** * Lock a given text submission that still needs to be assessed to prevent other tutors from receiving and assessing it. * - * @param textSubmission textSubmission to be locked - * @param correctionRound get submission with results in the correction round + * @param textSubmissionId id of the textSubmission to be locked + * @param correctionRound get submission with results in the correction round + * @return the locked textSubmission */ - public void lockTextSubmissionToBeAssessed(TextSubmission textSubmission, int correctionRound) { + public TextSubmission lockTextSubmissionToBeAssessed(long textSubmissionId, int correctionRound) { + // NOTE: we load the feedback for the submission eagerly to avoid org.hibernate.LazyInitializationException + final var textSubmission = textSubmissionRepository.findByIdWithEagerResultsAndFeedbackAndTextBlocksElseThrow(textSubmissionId); lockSubmission(textSubmission, correctionRound); + return textSubmission; } public TextSubmission findOneWithEagerResultFeedbackAndTextBlocks(Long submissionId) { diff --git a/src/main/java/de/tum/cit/aet/artemis/text/web/TextAssessmentResource.java b/src/main/java/de/tum/cit/aet/artemis/text/web/TextAssessmentResource.java index 900347ecc54e..0f1df354c04b 100644 --- a/src/main/java/de/tum/cit/aet/artemis/text/web/TextAssessmentResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/text/web/TextAssessmentResource.java @@ -348,14 +348,14 @@ public ResponseEntity deleteAssessment(@PathVariable Long participationId, * @param submissionId the id of the submission we want * @param correctionRound correction round for which we want the submission * @param resultId if result already exists, we want to get the submission for this specific result - * @return a Participation of the tutor in the submission + * @return a Participation with relevant data for a tutor or instructor to assess the submission */ @GetMapping("text-submissions/{submissionId}/for-assessment") @EnforceAtLeastTutor public ResponseEntity retrieveParticipationForSubmission(@PathVariable Long submissionId, @RequestParam(value = "correction-round", defaultValue = "0") int correctionRound, @RequestParam(value = "resultId", required = false) Long resultId) { log.debug("REST request to get data for tutors text assessment submission: {}", submissionId); - final var textSubmission = textSubmissionRepository.findByIdWithParticipationExerciseResultAssessorAssessmentNoteElseThrow(submissionId); + var textSubmission = textSubmissionRepository.findByIdWithParticipationExerciseResultAssessorAssessmentNoteElseThrow(submissionId); final Participation participation = textSubmission.getParticipation(); final var exercise = participation.getExercise(); final User user = userRepository.getUserWithGroupsAndAuthorities(); @@ -387,7 +387,9 @@ public ResponseEntity retrieveParticipationForSubmission(@PathVar .build(); } - textSubmissionService.lockTextSubmissionToBeAssessed(textSubmission, correctionRound); + textSubmission = textSubmissionService.lockTextSubmissionToBeAssessed(textSubmission.getId(), correctionRound); + // reconnect with participation + textSubmission.setParticipation(participation); // set it since it has changed result = textSubmission.getResultForCorrectionRound(correctionRound); } diff --git a/src/main/java/de/tum/cit/aet/artemis/text/web/TextSubmissionResource.java b/src/main/java/de/tum/cit/aet/artemis/text/web/TextSubmissionResource.java index 359077783330..b25506a90aef 100644 --- a/src/main/java/de/tum/cit/aet/artemis/text/web/TextSubmissionResource.java +++ b/src/main/java/de/tum/cit/aet/artemis/text/web/TextSubmissionResource.java @@ -166,7 +166,7 @@ private ResponseEntity handleTextSubmission(long exerciseId, Tex @EnforceAtLeastStudent public ResponseEntity getTextSubmissionWithResults(@PathVariable long submissionId) { log.debug("REST request to get text submission: {}", submissionId); - var textSubmission = textSubmissionRepository.findWithEagerResultsById(submissionId).orElseThrow(() -> new EntityNotFoundException("TextSubmission", submissionId)); + var textSubmission = textSubmissionRepository.findWithEagerResultsAssessorById(submissionId).orElseThrow(() -> new EntityNotFoundException("TextSubmission", submissionId)); if (!authCheckService.isAtLeastTeachingAssistantForExercise(textSubmission.getParticipation().getExercise())) { // anonymize and throw exception if not authorized to view submission @@ -219,7 +219,7 @@ public ResponseEntity getTextSubmissionWithoutAssessment(@PathVa } // Check if tutors can start assessing the students submission - this.textSubmissionService.checkIfExerciseDueDateIsReached(exercise); + textSubmissionService.checkIfExerciseDueDateIsReached(exercise); // Check if the limit of simultaneously locked submissions has been reached textSubmissionService.checkSubmissionLockLimit(exercise.getCourseViaExerciseGroupOrCourseMember().getId()); @@ -232,10 +232,10 @@ public ResponseEntity getTextSubmissionWithoutAssessment(@PathVa return ResponseEntity.ok(null); } - final TextSubmission textSubmission = optionalTextSubmission.get(); + TextSubmission textSubmission = optionalTextSubmission.get(); if (lockSubmission) { - textSubmissionService.lockTextSubmissionToBeAssessed(optionalTextSubmission.get(), correctionRound); + textSubmission = textSubmissionService.lockTextSubmissionToBeAssessed(textSubmission.getId(), correctionRound); textAssessmentService.prepareSubmissionForAssessment(textSubmission, textSubmission.getResultForCorrectionRound(correctionRound)); }