Feedback from Johannes

ls1intum · Oct 15, 2024 · 6812ddb · 6812ddb
1 parent 9a2dc2c
commit 6812ddb
Show file tree

Hide file tree

Showing 7 changed files with 37 additions and 49 deletions.
diff --git a/src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisChatSubSettings.java b/src/main/java/de/tum/cit/aet/artemis/iris/domain/settings/IrisChatSubSettings.java
@@ -28,7 +28,6 @@ public class IrisChatSubSettings extends IrisSubSettings {
     @Column(name = "rate_limit_timeframe_hours")
     private Integer rateLimitTimeframeHours;
 
-    @Nullable
     @Column(name = "enabled_for_categories")
     @Convert(converter = IrisListConverter.class)
     private SortedSet<String> enabledForCategories = new TreeSet<>();

diff --git a/src/main/java/de/tum/cit/aet/artemis/iris/dto/IrisCombinedChatSubSettingsDTO.java b/src/main/java/de/tum/cit/aet/artemis/iris/dto/IrisCombinedChatSubSettingsDTO.java
@@ -1,13 +1,13 @@
 package de.tum.cit.aet.artemis.iris.dto;
 
-import java.util.Set;
+import java.util.SortedSet;
 
 import jakarta.annotation.Nullable;
 
 import com.fasterxml.jackson.annotation.JsonInclude;
 
 @JsonInclude(JsonInclude.Include.NON_EMPTY)
-public record IrisCombinedChatSubSettingsDTO(boolean enabled, Integer rateLimit, Integer rateLimitTimeframeHours, @Nullable Set<String> allowedVariants,
-        @Nullable String selectedVariant, @Nullable Set<String> enabledForCategories) {
+public record IrisCombinedChatSubSettingsDTO(boolean enabled, Integer rateLimit, Integer rateLimitTimeframeHours, @Nullable SortedSet<String> allowedVariants,
+        @Nullable String selectedVariant, @Nullable SortedSet<String> enabledForCategories) {
 
 }
diff --git a/...main/java/de/tum/cit/aet/artemis/iris/dto/IrisCombinedTextExerciseChatSubSettingsDTO.java b/...main/java/de/tum/cit/aet/artemis/iris/dto/IrisCombinedTextExerciseChatSubSettingsDTO.java
@@ -1,13 +1,13 @@
 package de.tum.cit.aet.artemis.iris.dto;
 
-import java.util.Set;
+import java.util.SortedSet;
 
 import jakarta.annotation.Nullable;
 
 import com.fasterxml.jackson.annotation.JsonInclude;
 
 @JsonInclude(JsonInclude.Include.NON_EMPTY)
-public record IrisCombinedTextExerciseChatSubSettingsDTO(boolean enabled, Integer rateLimit, Integer rateLimitTimeframeHours, @Nullable Set<String> allowedVariants,
-        @Nullable String selectedVariant, @Nullable Set<String> enabledForCategories) {
+public record IrisCombinedTextExerciseChatSubSettingsDTO(boolean enabled, Integer rateLimit, Integer rateLimitTimeframeHours, @Nullable SortedSet<String> allowedVariants,
+        @Nullable String selectedVariant, @Nullable SortedSet<String> enabledForCategories) {
 
 }
diff --git a/src/main/java/de/tum/cit/aet/artemis/iris/service/settings/IrisSettingsService.java b/src/main/java/de/tum/cit/aet/artemis/iris/service/settings/IrisSettingsService.java
@@ -303,7 +303,7 @@ private IrisCourseSettings updateCourseSettings(IrisCourseSettings existingSetti
         // Automatically update the exercise settings when the enabledForCategories is changed
         var newEnabledForCategoriesExerciseChat = existingSettings.getIrisChatSettings() == null ? new TreeSet<String>()
                 : existingSettings.getIrisChatSettings().getEnabledForCategories();
-        if (!Objects.equals(oldEnabledForCategoriesExerciseChat, newEnabledForCategoriesExerciseChat)) {
+        if (!oldEnabledForCategoriesExerciseChat.equals(newEnabledForCategoriesExerciseChat)) {
             programmingExerciseRepository.findAllWithCategoriesByCourseId(existingSettings.getCourse().getId())
                     .forEach(exercise -> setEnabledForExerciseByCategories(exercise, oldEnabledForCategoriesExerciseChat, newEnabledForCategoriesExerciseChat));
         }
@@ -329,29 +329,15 @@ private IrisCourseSettings updateCourseSettings(IrisCourseSettings existingSetti
      * @param newEnabledForCategories The new enabled categories
      */
     public void setEnabledForExerciseByCategories(Exercise exercise, SortedSet<String> oldEnabledForCategories, SortedSet<String> newEnabledForCategories) {
-        var removedCategories = new TreeSet<>(oldEnabledForCategories == null ? Set.of() : oldEnabledForCategories);
+        var removedCategories = new TreeSet<>(oldEnabledForCategories);
         removedCategories.removeAll(newEnabledForCategories);
         var categories = getCategoryNames(exercise.getCategories());
 
-        if (newEnabledForCategories != null && categories.stream().anyMatch(newEnabledForCategories::contains)) {
-            var exerciseSettings = getRawIrisSettingsFor(exercise);
-            if (exercise instanceof ProgrammingExercise) {
-                exerciseSettings.getIrisChatSettings().setEnabled(true);
-            }
-            else if (exercise instanceof TextExercise) {
-                exerciseSettings.getIrisTextExerciseChatSettings().setEnabled(true);
-            }
-            irisSettingsRepository.save(exerciseSettings);
+        if (categories.stream().anyMatch(newEnabledForCategories::contains)) {
+            setExerciseSettingsEnabled(exercise, true);
         }
         else if (categories.stream().anyMatch(removedCategories::contains)) {
-            var exerciseSettings = getRawIrisSettingsFor(exercise);
-            if (exercise instanceof ProgrammingExercise) {
-                exerciseSettings.getIrisChatSettings().setEnabled(false);
-            }
-            else if (exercise instanceof TextExercise) {
-                exerciseSettings.getIrisTextExerciseChatSettings().setEnabled(false);
-            }
-            irisSettingsRepository.save(exerciseSettings);
+            setExerciseSettingsEnabled(exercise, false);
         }
     }
 
@@ -388,25 +374,29 @@ else if (exercise instanceof TextExercise) {
         }
 
         if (newCategories.stream().anyMatch(enabledForCategories::contains)) {
-            var exerciseSettings = getRawIrisSettingsFor(exercise);
-            if (exercise instanceof ProgrammingExercise) {
-                exerciseSettings.getIrisChatSettings().setEnabled(true);
-            }
-            else if (exercise instanceof TextExercise) {
-                exerciseSettings.getIrisTextExerciseChatSettings().setEnabled(true);
-            }
-            irisSettingsRepository.save(exerciseSettings);
+            setExerciseSettingsEnabled(exercise, true);
         }
         else if (oldCategories.stream().anyMatch(enabledForCategories::contains)) {
-            var exerciseSettings = getRawIrisSettingsFor(exercise);
-            if (exercise instanceof ProgrammingExercise) {
-                exerciseSettings.getIrisChatSettings().setEnabled(false);
-            }
-            else if (exercise instanceof TextExercise) {
-                exerciseSettings.getIrisTextExerciseChatSettings().setEnabled(false);
-            }
-            irisSettingsRepository.save(exerciseSettings);
+            setExerciseSettingsEnabled(exercise, false);
+        }
+    }
+
+    /**
+     * Helper method to set the enabled status for an exercise's Iris settings.
+     * Currently able to handle {@link ProgrammingExercise} and {@link TextExercise} settings.
+     *
+     * @param exercise The exercise to update the enabled status for
+     * @param enabled  Whether the Iris settings should be enabled
+     */
+    private void setExerciseSettingsEnabled(Exercise exercise, boolean enabled) {
+        var exerciseSettings = getRawIrisSettingsFor(exercise);
+        if (exercise instanceof ProgrammingExercise) {
+            exerciseSettings.getIrisChatSettings().setEnabled(enabled);
+        }
+        else if (exercise instanceof TextExercise) {
+            exerciseSettings.getIrisTextExerciseChatSettings().setEnabled(enabled);
         }
+        irisSettingsRepository.save(exerciseSettings);
     }
 
     /**

diff --git a/src/main/java/de/tum/cit/aet/artemis/iris/service/settings/IrisSubSettingsService.java b/src/main/java/de/tum/cit/aet/artemis/iris/service/settings/IrisSubSettingsService.java
@@ -332,7 +332,7 @@ private Integer getCombinedRateLimit(List<IrisSettings> settingsList) {
      * @param subSettingsFunction Function to get the sub settings from an IrisSettings object.
      * @return Combined allowedVariants field.
      */
-    private Set<String> getCombinedAllowedVariants(List<IrisSettings> settingsList, Function<IrisSettings, IrisSubSettings> subSettingsFunction) {
+    private SortedSet<String> getCombinedAllowedVariants(List<IrisSettings> settingsList, Function<IrisSettings, IrisSubSettings> subSettingsFunction) {
         return settingsList.stream().filter(Objects::nonNull).map(subSettingsFunction).filter(Objects::nonNull).map(IrisSubSettings::getAllowedVariants).filter(Objects::nonNull)
                 .filter(models -> !models.isEmpty()).reduce((first, second) -> second).orElse(new TreeSet<>());
     }
@@ -350,7 +350,7 @@ private String getCombinedSelectedVariant(List<IrisSettings> settingsList, Funct
                 .filter(model -> model != null && !model.isBlank()).reduce((first, second) -> second).orElse(null);
     }
 
-    private Set<String> getCombinedEnabledForCategories(List<IrisSettings> settingsList, Function<IrisSettings, IrisChatSubSettings> subSettingsFunction) {
+    private SortedSet<String> getCombinedEnabledForCategories(List<IrisSettings> settingsList, Function<IrisSettings, IrisChatSubSettings> subSettingsFunction) {
         return settingsList.stream().filter(Objects::nonNull).filter(settings -> settings instanceof IrisCourseSettings).map(subSettingsFunction).filter(Objects::nonNull)
                 .map(IrisChatSubSettings::getEnabledForCategories).filter(Objects::nonNull).filter(models -> !models.isEmpty()).reduce((first, second) -> second)
                 .orElse(new TreeSet<>());

diff --git a/...ain/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseRepository.java b/...ain/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingExerciseRepository.java
@@ -109,7 +109,7 @@ Optional<ProgrammingExercise> findWithTemplateAndSolutionParticipationTeamAssign
     Optional<ProgrammingExercise> findWithAuxiliaryRepositoriesById(long exerciseId);
 
     @EntityGraph(type = LOAD, attributePaths = { "auxiliaryRepositories", "competencies", "buildConfig", "categories" })
-    Optional<ProgrammingExercise> findWithAuxiliaryRepositoriesCompetenciesAndBuildConfigById(long exerciseId);
+    Optional<ProgrammingExercise> findForUpdateById(long exerciseId);
 
     @EntityGraph(type = LOAD, attributePaths = "submissionPolicy")
     Optional<ProgrammingExercise> findWithSubmissionPolicyById(long exerciseId);
@@ -599,8 +599,8 @@ default ProgrammingExercise findByIdWithAuxiliaryRepositoriesElseThrow(long prog
      * @return The programming exercise related to the given id
      */
     @NotNull
-    default ProgrammingExercise findByIdWithAuxiliaryRepositoriesCompetenciesAndBuildConfigElseThrow(long programmingExerciseId) throws EntityNotFoundException {
-        return getValueElseThrow(findWithAuxiliaryRepositoriesCompetenciesAndBuildConfigById(programmingExerciseId), programmingExerciseId);
+    default ProgrammingExercise findForUpdateByIdElseThrow(long programmingExerciseId) throws EntityNotFoundException {
+        return getValueElseThrow(findForUpdateById(programmingExerciseId), programmingExerciseId);
     }
 
     /**

diff --git a/src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java b/src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java
@@ -298,8 +298,7 @@ public ResponseEntity<ProgrammingExercise> updateProgrammingExercise(@RequestBod
 
         checkProgrammingExerciseForError(updatedProgrammingExercise);
 
-        var programmingExerciseBeforeUpdate = programmingExerciseRepository
-                .findByIdWithAuxiliaryRepositoriesCompetenciesAndBuildConfigElseThrow(updatedProgrammingExercise.getId());
+        var programmingExerciseBeforeUpdate = programmingExerciseRepository.findForUpdateByIdElseThrow(updatedProgrammingExercise.getId());
         if (!Objects.equals(programmingExerciseBeforeUpdate.getShortName(), updatedProgrammingExercise.getShortName())) {
             throw new BadRequestAlertException("The programming exercise short name cannot be changed", ENTITY_NAME, "shortNameCannotChange");
         }