Adaptive learning: Improve import of competencies when some already e…

…xist (#9774)
ls1intum · Nov 16, 2024 · 7bf8bcd · 7bf8bcd
1 parent 0ffa44f
commit 7bf8bcd
Show file tree

Hide file tree

Showing 9 changed files with 74 additions and 48 deletions.
diff --git a/src/main/java/de/tum/cit/aet/artemis/atlas/service/competency/CompetencyService.java b/src/main/java/de/tum/cit/aet/artemis/atlas/service/competency/CompetencyService.java
@@ -2,9 +2,7 @@
 
 import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_CORE;
 
-import java.util.ArrayList;
 import java.util.Collection;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -68,17 +66,7 @@ public CompetencyService(CompetencyRepository competencyRepository, Authorizatio
      * @return The set of imported competencies, each also containing the relations it is the tail competency for.
      */
     public Set<CompetencyWithTailRelationDTO> importCompetencies(Course course, Collection<? extends CourseCompetency> competencies, CompetencyImportOptionsDTO importOptions) {
-        var idToImportedCompetency = new HashMap<Long, CompetencyWithTailRelationDTO>();
-
-        for (var competency : competencies) {
-            Competency importedCompetency = new Competency(competency);
-            importedCompetency.setCourse(course);
-
-            importedCompetency = competencyRepository.save(importedCompetency);
-            idToImportedCompetency.put(competency.getId(), new CompetencyWithTailRelationDTO(importedCompetency, new ArrayList<>()));
-        }
-
-        return importCourseCompetencies(course, competencies, idToImportedCompetency, importOptions);
+        return importCourseCompetencies(course, competencies, importOptions, Competency::new);
     }
 
     /**

diff --git a/src/main/java/de/tum/cit/aet/artemis/atlas/service/competency/CourseCompetencyService.java b/src/main/java/de/tum/cit/aet/artemis/atlas/service/competency/CourseCompetencyService.java
@@ -8,7 +8,6 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
-import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.Function;
@@ -213,34 +212,39 @@ public void filterOutLearningObjectsThatUserShouldNotSee(CourseCompetency compet
      * @return The set of imported course competencies, each also containing the relations it is the tail competency for.
      */
     public Set<CompetencyWithTailRelationDTO> importCourseCompetencies(Course course, Collection<CourseCompetency> courseCompetencies, CompetencyImportOptionsDTO importOptions) {
-        var idToImportedCompetency = new HashMap<Long, CompetencyWithTailRelationDTO>();
-
-        for (var courseCompetency : courseCompetencies) {
-            CourseCompetency importedCompetency = switch (courseCompetency) {
-                case Competency competency -> new Competency(competency);
-                case Prerequisite prerequisite -> new Prerequisite(prerequisite);
-                default -> throw new IllegalStateException("Unexpected value: " + courseCompetency);
-            };
-            importedCompetency.setCourse(course);
+        Function<CourseCompetency, CourseCompetency> createNewCourseCompetency = courseCompetency -> switch (courseCompetency) {
+            case Competency competency -> new Competency(competency);
+            case Prerequisite prerequisite -> new Prerequisite(prerequisite);
+            default -> throw new IllegalStateException("Unexpected value: " + courseCompetency);
+        };
 
-            importedCompetency = courseCompetencyRepository.save(importedCompetency);
-            idToImportedCompetency.put(courseCompetency.getId(), new CompetencyWithTailRelationDTO(importedCompetency, new ArrayList<>()));
-        }
-
-        return importCourseCompetencies(course, courseCompetencies, idToImportedCompetency, importOptions);
+        return importCourseCompetencies(course, courseCompetencies, importOptions, createNewCourseCompetency);
     }
 
     /**
      * Imports the given competencies and relations into a course
      *
-     * @param course                 the course to import into
-     * @param competenciesToImport   the source competencies that were imported
-     * @param idToImportedCompetency map of original competency id to imported competency
-     * @param importOptions          the import options
+     * @param course                    the course to import into
+     * @param competenciesToImport      the source competencies that were imported
+     * @param importOptions             the import options
+     * @param createNewCourseCompetency the function that creates new course competencies
      * @return The set of imported competencies, each also containing the relations it is the tail competency for.
      */
     public Set<CompetencyWithTailRelationDTO> importCourseCompetencies(Course course, Collection<? extends CourseCompetency> competenciesToImport,
-            Map<Long, CompetencyWithTailRelationDTO> idToImportedCompetency, CompetencyImportOptionsDTO importOptions) {
+            CompetencyImportOptionsDTO importOptions, Function<CourseCompetency, CourseCompetency> createNewCourseCompetency) {
+        var idToImportedCompetency = new HashMap<Long, CompetencyWithTailRelationDTO>();
+
+        Set<CourseCompetency> competenciesInCourse = courseCompetencyRepository.findAllForCourse(course.getId());
+
+        for (var courseCompetency : competenciesToImport) {
+            Optional<CourseCompetency> existingCompetency = competenciesInCourse.stream().filter(competency -> competency.getTitle().equals(courseCompetency.getTitle()))
+                    .filter(competency -> competency.getType().equals(courseCompetency.getType())).findFirst();
+            CourseCompetency importedCompetency = existingCompetency.orElse(createNewCourseCompetency.apply(courseCompetency));
+            importedCompetency.setCourse(course);
+            idToImportedCompetency.put(courseCompetency.getId(), new CompetencyWithTailRelationDTO(importedCompetency, new ArrayList<>()));
+        }
+        courseCompetencyRepository.saveAll(idToImportedCompetency.values().stream().map(CompetencyWithTailRelationDTO::competency).toList());
+
         if (course.getLearningPathsEnabled()) {
             var importedCompetencies = idToImportedCompetency.values().stream().map(CompetencyWithTailRelationDTO::competency).toList();
             learningPathService.linkCompetenciesToLearningPathsOfCourse(importedCompetencies, course.getId());

diff --git a/src/main/java/de/tum/cit/aet/artemis/atlas/service/competency/PrerequisiteService.java b/src/main/java/de/tum/cit/aet/artemis/atlas/service/competency/PrerequisiteService.java
@@ -2,9 +2,7 @@
 
 import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_CORE;
 
-import java.util.ArrayList;
 import java.util.Collection;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Set;
 
@@ -59,17 +57,7 @@ public PrerequisiteService(PrerequisiteRepository prerequisiteRepository, Author
      * @return The set of imported prerequisites, each also containing the relations for which it is the tail prerequisite for.
      */
     public Set<CompetencyWithTailRelationDTO> importPrerequisites(Course course, Collection<? extends CourseCompetency> prerequisites, CompetencyImportOptionsDTO importOptions) {
-        var idToImportedPrerequisite = new HashMap<Long, CompetencyWithTailRelationDTO>();
-
-        for (var prerequisite : prerequisites) {
-            Prerequisite importedPrerequisite = new Prerequisite(prerequisite);
-            importedPrerequisite.setCourse(course);
-
-            importedPrerequisite = prerequisiteRepository.save(importedPrerequisite);
-            idToImportedPrerequisite.put(prerequisite.getId(), new CompetencyWithTailRelationDTO(importedPrerequisite, new ArrayList<>()));
-        }
-
-        return importCourseCompetencies(course, prerequisites, idToImportedPrerequisite, importOptions);
+        return importCourseCompetencies(course, prerequisites, importOptions, Prerequisite::new);
     }
 
     /**

diff --git a/...pp/app/course/competencies/competency-management/competency-management-table.component.ts b/...pp/app/course/competencies/competency-management/competency-management-table.component.ts
@@ -95,7 +95,8 @@ export class CompetencyManagementTableComponent implements OnInit, OnDestroy {
      */
     updateDataAfterImportAll(res: Array<CompetencyWithTailRelationDTO>) {
         const importedCompetencies = res.map((dto) => dto.competency).filter((element): element is CourseCompetency => !!element);
-        this.courseCompetencies.push(...importedCompetencies);
+        const newCourseCompetencies = importedCompetencies.filter((competency) => !this.courseCompetencies.some((existingCompetency) => existingCompetency.id === competency.id));
+        this.courseCompetencies.push(...newCourseCompetencies);
         this.allCompetencies.update((allCourseCompetencies) => allCourseCompetencies.concat(importedCompetencies));
     }
 

diff --git a/...n/webapp/app/course/competencies/competency-management/competency-management.component.ts b/...n/webapp/app/course/competencies/competency-management/competency-management.component.ts
@@ -155,7 +155,10 @@ export class CompetencyManagementComponent implements OnInit {
      */
     updateDataAfterImportAll(res: Array<CompetencyWithTailRelationDTO>) {
         const importedCourseCompetencies = res.map((dto) => dto.competency!);
-        this.courseCompetencies.update((courseCompetencies) => courseCompetencies.concat(importedCourseCompetencies));
+        const newCourseCompetencies = importedCourseCompetencies.filter(
+            (competency) => !this.courseCompetencies().some((existingCompetency) => existingCompetency.id === competency.id),
+        );
+        this.courseCompetencies.update((courseCompetencies) => courseCompetencies.concat(newCourseCompetencies));
     }
 
     onRemoveCompetency(competencyId: number) {

diff --git a/...e/tum/cit/aet/artemis/atlas/competency/AbstractCompetencyPrerequisiteIntegrationTest.java b/...e/tum/cit/aet/artemis/atlas/competency/AbstractCompetencyPrerequisiteIntegrationTest.java
@@ -2,6 +2,7 @@
 
 import static de.tum.cit.aet.artemis.core.util.TestResourceUtils.HalfSecond;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Fail.fail;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 
@@ -16,6 +17,7 @@
 import org.springframework.http.HttpStatus;
 
 import de.tum.cit.aet.artemis.atlas.AbstractAtlasIntegrationTest;
+import de.tum.cit.aet.artemis.atlas.domain.competency.Competency;
 import de.tum.cit.aet.artemis.atlas.domain.competency.CompetencyExerciseLink;
 import de.tum.cit.aet.artemis.atlas.domain.competency.CompetencyLectureUnitLink;
 import de.tum.cit.aet.artemis.atlas.domain.competency.CompetencyRelation;
@@ -27,6 +29,7 @@
 import de.tum.cit.aet.artemis.atlas.dto.CompetencyImportResponseDTO;
 import de.tum.cit.aet.artemis.atlas.dto.CompetencyWithTailRelationDTO;
 import de.tum.cit.aet.artemis.core.domain.Course;
+import de.tum.cit.aet.artemis.core.domain.DomainObject;
 import de.tum.cit.aet.artemis.core.domain.User;
 import de.tum.cit.aet.artemis.exercise.domain.ExerciseMode;
 import de.tum.cit.aet.artemis.exercise.domain.IncludedInOverallScore;
@@ -511,6 +514,27 @@ void shouldImportAllCompetencies(Function<Course, CourseCompetency> createCourse
         assertThat(competencyDTOList.get(1).tailRelations()).isNull();
     }
 
+    // Test
+    void shouldImportAllCompetenciesWithSomeExisting(Function<CourseCompetency, CourseCompetency> copyCourseCompetency, int expectedCourseCompetencies) throws Exception {
+        competencyUtilService.createCompetencies(course, 2);
+        prerequisiteUtilService.createPrerequisites(course, 2);
+
+        CourseCompetency newCompetency = copyCourseCompetency.apply(courseCompetency);
+        newCompetency.setCourse(course2);
+        newCompetency = courseCompetencyRepository.save(newCompetency);
+
+        CompetencyImportOptionsDTO importOptions = new CompetencyImportOptionsDTO(Set.of(), Optional.of(course.getId()), false, false, false, Optional.empty(), false);
+        importAllCall(course2.getId(), importOptions, HttpStatus.CREATED);
+
+        course2 = courseRepository.findByIdWithExercisesAndLecturesAndLectureUnitsAndCompetenciesElseThrow(course2.getId());
+        assertThat(course2.getPrerequisites().size() + course2.getCompetencies().size()).isEqualTo(expectedCourseCompetencies);
+        switch (newCompetency) {
+            case Competency competency -> assertThat(course2.getCompetencies().stream().map(DomainObject::getId)).contains(competency.getId());
+            case Prerequisite prerequisite -> assertThat(course2.getPrerequisites().stream().map(DomainObject::getId)).contains(prerequisite.getId());
+            default -> fail("Unexpected CourseCompetency subclass");
+        }
+    }
+
     // Test
     void shouldImportAllExerciseAndLectureWithCompetency() throws Exception {
         createProgrammingExercise(ZonedDateTime.now(), ZonedDateTime.now());

diff --git a/src/test/java/de/tum/cit/aet/artemis/atlas/competency/CompetencyIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/atlas/competency/CompetencyIntegrationTest.java
@@ -284,6 +284,12 @@ void shouldImportAllCompetencies() throws Exception {
         super.shouldImportAllCompetencies(competencyUtilService::createCompetency);
     }
 
+    @Test
+    @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
+    void shouldImportAllCompetenciesWithSomeExisting() throws Exception {
+        shouldImportAllCompetenciesWithSomeExisting(Competency::new, 3);
+    }
+
     @Test
     @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
     void shouldImportAllExerciseAndLectureWithCompetency() throws Exception {

diff --git a/src/test/java/de/tum/cit/aet/artemis/atlas/competency/CourseCompetencyIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/atlas/competency/CourseCompetencyIntegrationTest.java
@@ -194,6 +194,12 @@ void shouldImportAllCompetencies() throws Exception {
         super.shouldImportAllCompetencies(competencyUtilService::createCompetency);
     }
 
+    @Test
+    @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
+    void shouldImportAllCompetenciesWithSomeExisting() throws Exception {
+        shouldImportAllCompetenciesWithSomeExisting(Competency::new, 5);
+    }
+
     @Test
     @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
     void shouldImportAllExerciseAndLectureWithCompetency() throws Exception {

diff --git a/src/test/java/de/tum/cit/aet/artemis/atlas/competency/PrerequisiteIntegrationTest.java b/src/test/java/de/tum/cit/aet/artemis/atlas/competency/PrerequisiteIntegrationTest.java
@@ -278,6 +278,12 @@ void shouldImportAllCompetencies() throws Exception {
         super.shouldImportAllCompetencies(prerequisiteUtilService::createPrerequisite);
     }
 
+    @Test
+    @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
+    void shouldImportAllCompetenciesWithSomeExisting() throws Exception {
+        shouldImportAllCompetenciesWithSomeExisting(Prerequisite::new, 3);
+    }
+
     @Test
     @WithMockUser(username = TEST_PREFIX + "instructor1", roles = "INSTRUCTOR")
     void shouldImportAllExerciseAndLectureWithCompetency() throws Exception {