From 6a666eb36ec25ec8786ec5fa794804ff7f42cfa0 Mon Sep 17 00:00:00 2001 From: Stephan Krusche Date: Sat, 23 Dec 2023 18:32:58 +0100 Subject: [PATCH] Development: Improve course overview and course details performance (#7821) --- jest.config.js | 2 +- .../de/tum/in/www1/artemis/domain/Course.java | 44 ++++++++++++++ .../CompetencyProgressRepository.java | 14 ++++- .../CompetencyRelationRepository.java | 2 +- .../repository/CompetencyRepository.java | 13 +++- .../artemis/repository/CourseRepository.java | 9 +-- .../TutorialGroupFreePeriodRepository.java | 10 ++-- .../TutorialGroupRepository.java | 25 +++++--- .../TutorialGroupSessionRepository.java | 59 +++++++++++-------- ...TutorialGroupsConfigurationRepository.java | 14 +++-- .../www1/artemis/service/CourseService.java | 12 ++++ .../learningpath/LearningPathNgxService.java | 2 +- .../tutorialgroups/TutorialGroupService.java | 23 +++----- .../artemis/web/rest/CompetencyResource.java | 12 +++- .../www1/artemis/web/rest/CourseResource.java | 41 +++++++------ src/main/webapp/app/entities/course.model.ts | 6 ++ .../app/overview/course-card.component.ts | 4 +- .../course-competencies.component.ts | 23 ++++---- .../app/overview/course-overview.component.ts | 4 +- .../www1/artemis/DatabaseQueryCountTest.java | 3 +- .../course/course-overview.component.spec.ts | 11 +--- 21 files changed, 218 insertions(+), 115 deletions(-) diff --git a/jest.config.js b/jest.config.js index 8a44bd317ae1..8f73f77fbd5e 100644 --- a/jest.config.js +++ b/jest.config.js @@ -102,7 +102,7 @@ module.exports = { // TODO: in the future, the following values should increase to at least 90% statements: 86.7, branches: 73.7, - functions: 80.7, + functions: 80.8, lines: 86.7, }, }, diff --git a/src/main/java/de/tum/in/www1/artemis/domain/Course.java b/src/main/java/de/tum/in/www1/artemis/domain/Course.java index ce8e9c22bdec..c206dcff222b 100644 --- a/src/main/java/de/tum/in/www1/artemis/domain/Course.java +++ b/src/main/java/de/tum/in/www1/artemis/domain/Course.java @@ -267,6 +267,50 @@ public class Course extends DomainObject { @Transient private Long numberOfStudentsTransient; + @Transient + private Long numberOfLecturesTransient; + + @Transient + private Long numberOfTutorialGroupsTransient; + + @Transient + private Long numberOfCompetenciesTransient; + + @Transient + private Long numberOfPrerequisitesTransient; + + public Long getNumberOfLectures() { + return numberOfLecturesTransient; + } + + public Long getNumberOfTutorialGroups() { + return numberOfTutorialGroupsTransient; + } + + public Long getNumberOfCompetencies() { + return numberOfCompetenciesTransient; + } + + public Long getNumberOfPrerequisites() { + return numberOfPrerequisitesTransient; + } + + public void setNumberOfLectures(Long numberOfLectures) { + this.numberOfLecturesTransient = numberOfLectures; + } + + public void setNumberOfTutorialGroups(Long numberOfTutorialGroups) { + this.numberOfTutorialGroupsTransient = numberOfTutorialGroups; + } + + public void setNumberOfCompetencies(Long numberOfCompetencies) { + this.numberOfCompetenciesTransient = numberOfCompetencies; + } + + public void setNumberOfPrerequisites(Long numberOfPrerequisites) { + this.numberOfPrerequisitesTransient = numberOfPrerequisites; + } + public String getTitle() { return title; } diff --git a/src/main/java/de/tum/in/www1/artemis/repository/CompetencyProgressRepository.java b/src/main/java/de/tum/in/www1/artemis/repository/CompetencyProgressRepository.java index 6ec922248bb8..86ac55a9ab75 100644 --- a/src/main/java/de/tum/in/www1/artemis/repository/CompetencyProgressRepository.java +++ b/src/main/java/de/tum/in/www1/artemis/repository/CompetencyProgressRepository.java @@ -1,8 +1,6 @@ package de.tum.in.www1.artemis.repository; -import java.util.List; -import java.util.Optional; -import java.util.Set; +import java.util.*; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.Modifying; @@ -11,6 +9,7 @@ import org.springframework.stereotype.Repository; import org.springframework.transaction.annotation.Transactional; +import de.tum.in.www1.artemis.domain.competency.Competency; import de.tum.in.www1.artemis.domain.competency.CompetencyProgress; @Repository @@ -40,6 +39,15 @@ public interface CompetencyProgressRepository extends JpaRepository findByCompetencyIdAndUserId(@Param("competencyId") Long competencyId, @Param("userId") Long userId); + @Query(""" + SELECT cp + FROM CompetencyProgress cp + LEFT JOIN cp.learningGoal + WHERE cp.learningGoal in :competencies + AND cp.user.id = :userId + """) + Set findByCompetenciesAndUser(@Param("competencies") Collection competencies, @Param("userId") Long userId); + @Query(""" SELECT cp FROM CompetencyProgress cp diff --git a/src/main/java/de/tum/in/www1/artemis/repository/CompetencyRelationRepository.java b/src/main/java/de/tum/in/www1/artemis/repository/CompetencyRelationRepository.java index 2819f6202cfd..73099a2bf5ce 100644 --- a/src/main/java/de/tum/in/www1/artemis/repository/CompetencyRelationRepository.java +++ b/src/main/java/de/tum/in/www1/artemis/repository/CompetencyRelationRepository.java @@ -42,7 +42,7 @@ public interface CompetencyRelationRepository extends JpaRepository findAllByCourseId(@Param("courseId") Long courseId); + Set findAllWithHeadAndTailByCourseId(@Param("courseId") Long courseId); @Query(""" SELECT count(cr) diff --git a/src/main/java/de/tum/in/www1/artemis/repository/CompetencyRepository.java b/src/main/java/de/tum/in/www1/artemis/repository/CompetencyRepository.java index 5922bd496bc8..d4184506924e 100644 --- a/src/main/java/de/tum/in/www1/artemis/repository/CompetencyRepository.java +++ b/src/main/java/de/tum/in/www1/artemis/repository/CompetencyRepository.java @@ -27,8 +27,7 @@ public interface CompetencyRepository extends JpaRepository { @Query(""" SELECT c FROM Competency c - LEFT JOIN FETCH c.userProgress progress - WHERE c.course.id = :courseId + WHERE c.course.id = :courseId """) Set findAllForCourse(@Param("courseId") Long courseId); @@ -114,12 +113,20 @@ public interface CompetencyRepository extends JpaRepository { @Query(""" SELECT pr FROM Competency pr - LEFT JOIN FETCH pr.consecutiveCourses c + LEFT JOIN pr.consecutiveCourses c WHERE c.id = :courseId ORDER BY pr.title """) Set findPrerequisitesByCourseId(@Param("courseId") Long courseId); + @Query(""" + SELECT COUNT(*) + FROM Competency pr + LEFT JOIN pr.consecutiveCourses c + WHERE c.id = :courseId + """) + Long countPrerequisitesByCourseId(@Param("courseId") Long courseId); + /** * Query which fetches all competencies for which the user is editor or instructor in the course and * matching the search criteria. diff --git a/src/main/java/de/tum/in/www1/artemis/repository/CourseRepository.java b/src/main/java/de/tum/in/www1/artemis/repository/CourseRepository.java index df4e6eb2a7d0..3897a69476e9 100644 --- a/src/main/java/de/tum/in/www1/artemis/repository/CourseRepository.java +++ b/src/main/java/de/tum/in/www1/artemis/repository/CourseRepository.java @@ -134,19 +134,20 @@ SELECT CASE WHEN (count(c) > 0) THEN true ELSE false END @EntityGraph(type = LOAD, attributePaths = { "competencies", "learningPaths", "learningPaths.competencies" }) Optional findWithEagerLearningPathsAndCompetenciesById(long courseId); - @EntityGraph(type = LOAD, attributePaths = { "lectures" }) + // Note: we load attachments directly because otherwise, they will be loaded in subsequent DB calls due to the EAGER relationship + @EntityGraph(type = LOAD, attributePaths = { "lectures", "lectures.attachments" }) Optional findWithEagerLecturesById(long courseId); - @EntityGraph(type = LOAD, attributePaths = { "exercises", "lectures" }) + @EntityGraph(type = LOAD, attributePaths = { "exercises", "lectures", "lectures.attachments" }) Optional findWithEagerExercisesAndLecturesById(long courseId); - @EntityGraph(type = LOAD, attributePaths = { "lectures", "lectures.lectureUnits" }) + @EntityGraph(type = LOAD, attributePaths = { "lectures", "lectures.lectureUnits", "lectures.attachments" }) Optional findWithEagerLecturesAndLectureUnitsById(long courseId); @EntityGraph(type = LOAD, attributePaths = { "organizations", "competencies", "prerequisites", "tutorialGroupsConfiguration", "onlineCourseConfiguration" }) Optional findForUpdateById(long courseId); - @EntityGraph(type = LOAD, attributePaths = { "exercises", "lectures", "lectures.lectureUnits", "competencies", "prerequisites" }) + @EntityGraph(type = LOAD, attributePaths = { "exercises", "lectures", "lectures.lectureUnits", "lectures.attachments", "competencies", "prerequisites" }) Optional findWithEagerExercisesAndLecturesAndLectureUnitsAndCompetenciesById(long courseId); @Query(""" diff --git a/src/main/java/de/tum/in/www1/artemis/repository/tutorialgroups/TutorialGroupFreePeriodRepository.java b/src/main/java/de/tum/in/www1/artemis/repository/tutorialgroups/TutorialGroupFreePeriodRepository.java index 507a8231b298..f6052dba6dfb 100644 --- a/src/main/java/de/tum/in/www1/artemis/repository/tutorialgroups/TutorialGroupFreePeriodRepository.java +++ b/src/main/java/de/tum/in/www1/artemis/repository/tutorialgroups/TutorialGroupFreePeriodRepository.java @@ -17,10 +17,12 @@ public interface TutorialGroupFreePeriodRepository extends JpaRepository { @Query(""" - SELECT tutorialGroupFreePeriod - FROM TutorialGroupFreePeriod tutorialGroupFreePeriod - WHERE tutorialGroupFreePeriod.start <= :#{#toInclusive} AND tutorialGroupFreePeriod.end >= :#{#fromInclusive} - AND tutorialGroupFreePeriod.tutorialGroupsConfiguration.course = :#{#course}""") + SELECT period + FROM TutorialGroupFreePeriod period + WHERE period.start <= :#{#toInclusive} + AND period.end >= :#{#fromInclusive} + AND period.tutorialGroupsConfiguration.course = :#{#course} + """) Optional findOverlappingInSameCourse(@Param("course") Course course, @Param("fromInclusive") ZonedDateTime fromInclusive, @Param("toInclusive") ZonedDateTime toInclusive); diff --git a/src/main/java/de/tum/in/www1/artemis/repository/tutorialgroups/TutorialGroupRepository.java b/src/main/java/de/tum/in/www1/artemis/repository/tutorialgroups/TutorialGroupRepository.java index 756e1138eeb0..8fb98631646a 100644 --- a/src/main/java/de/tum/in/www1/artemis/repository/tutorialgroups/TutorialGroupRepository.java +++ b/src/main/java/de/tum/in/www1/artemis/repository/tutorialgroups/TutorialGroupRepository.java @@ -25,7 +25,7 @@ public interface TutorialGroupRepository extends JpaRepository getTutorialGroupWithChannel(@Param("tutorialGroupId") Long tutorialGroupId); @@ -83,19 +83,24 @@ WHERE tutorialGroup.course.instructorGroupName IN (:#{#userGroups}) AND tutorial @Query(""" SELECT tutorialGroup FROM TutorialGroup tutorialGroup - LEFT JOIN FETCH tutorialGroup.teachingAssistant - LEFT JOIN FETCH tutorialGroup.registrations + LEFT JOIN FETCH tutorialGroup.teachingAssistant + LEFT JOIN FETCH tutorialGroup.registrations + LEFT JOIN FETCH tutorialGroup.tutorialGroupSessions + LEFT JOIN FETCH tutorialGroup.tutorialGroupSchedule + LEFT JOIN FETCH tutorialGroup.tutorialGroupChannel WHERE tutorialGroup.course.id = :#{#courseId} - ORDER BY tutorialGroup.title""") - Set findAllByCourseIdWithTeachingAssistantAndRegistrations(@Param("courseId") Long courseId); + ORDER BY tutorialGroup.title + """) + Set findAllByCourseIdWithTeachingAssistantRegistrationsAndSchedule(@Param("courseId") Long courseId); @Query(""" SELECT tutorialGroup FROM TutorialGroup tutorialGroup - LEFT JOIN FETCH tutorialGroup.teachingAssistant - LEFT JOIN FETCH tutorialGroup.registrations - LEFT JOIN FETCH tutorialGroup.tutorialGroupSessions - LEFT JOIN FETCH tutorialGroup.tutorialGroupSchedule + LEFT JOIN FETCH tutorialGroup.teachingAssistant + LEFT JOIN FETCH tutorialGroup.registrations + LEFT JOIN FETCH tutorialGroup.tutorialGroupSessions + LEFT JOIN FETCH tutorialGroup.tutorialGroupSchedule + LEFT JOIN FETCH tutorialGroup.tutorialGroupChannel WHERE tutorialGroup.id = :#{#tutorialGroupId} """) Optional findByIdWithTeachingAssistantAndRegistrationsAndSessions(@Param("tutorialGroupId") long tutorialGroupId); @@ -161,4 +166,6 @@ default TutorialGroup findByIdWithTeachingAssistantAndRegistrationsAndSessionsEl default Optional getTutorialGroupChannel(Long tutorialGroupId) { return getTutorialGroupWithChannel(tutorialGroupId).map(TutorialGroup::getTutorialGroupChannel); } + + Long countByCourse(Course course); } diff --git a/src/main/java/de/tum/in/www1/artemis/repository/tutorialgroups/TutorialGroupSessionRepository.java b/src/main/java/de/tum/in/www1/artemis/repository/tutorialgroups/TutorialGroupSessionRepository.java index a91028839f2c..36041bcb9a69 100644 --- a/src/main/java/de/tum/in/www1/artemis/repository/tutorialgroups/TutorialGroupSessionRepository.java +++ b/src/main/java/de/tum/in/www1/artemis/repository/tutorialgroups/TutorialGroupSessionRepository.java @@ -22,49 +22,58 @@ public interface TutorialGroupSessionRepository extends JpaRepository { @Query(""" - SELECT tutorialGroupSession - FROM TutorialGroupSession tutorialGroupSession - WHERE tutorialGroupSession.tutorialGroup.id = :#{#tutorialGroupId} - AND tutorialGroupSession.status = :#{#status} - AND tutorialGroupSession.start >= :#{#now} - ORDER BY tutorialGroupSession.start""") + SELECT session + FROM TutorialGroupSession session + WHERE session.tutorialGroup.id = :#{#tutorialGroupId} + AND session.status = :#{#status} + AND session.start >= :#{#now} + ORDER BY session.start + """) List findNextSessionsOfStatus(@Param("tutorialGroupId") Long tutorialGroupId, @Param("now") ZonedDateTime now, @Param("status") TutorialGroupSessionStatus status); @Query(""" - SELECT tutorialGroupSession - FROM TutorialGroupSession tutorialGroupSession - WHERE tutorialGroupSession.tutorialGroup.id = :#{#tutorialGroupId}""") + SELECT session + FROM TutorialGroupSession session + WHERE session.tutorialGroup.id = :#{#tutorialGroupId} + """) Set findAllByTutorialGroupId(@Param("tutorialGroupId") Long tutorialGroupId); @Query(""" - SELECT tutorialGroupSession - FROM TutorialGroupSession tutorialGroupSession - WHERE tutorialGroupSession.tutorialGroupSchedule.id = :#{#scheduleId}""") + SELECT session + FROM TutorialGroupSession session + WHERE session.tutorialGroupSchedule.id = :#{#scheduleId} + """) Set findAllByScheduleId(@Param("scheduleId") Long scheduleId); @Query(""" - SELECT tutorialGroupSession - FROM TutorialGroupSession tutorialGroupSession - WHERE tutorialGroupSession.start <= :#{#end} AND tutorialGroupSession.end >= :#{#start} - AND tutorialGroupSession.tutorialGroup = :#{#tutorialGroup}""") + SELECT session + FROM TutorialGroupSession session + WHERE session.start <= :#{#end} + AND session.end >= :#{#start} + AND session.tutorialGroup = :#{#tutorialGroup} + """) Set findOverlappingInSameTutorialGroup(@Param("tutorialGroup") TutorialGroup tutorialGroup, @Param("start") ZonedDateTime start, @Param("end") ZonedDateTime end); @Query(""" - SELECT tutorialGroupSession - FROM TutorialGroupSession tutorialGroupSession - WHERE tutorialGroupSession.start <= :#{#end} AND tutorialGroupSession.end >= :#{#start} - AND tutorialGroupSession.tutorialGroupSchedule IS NULL - AND tutorialGroupSession.tutorialGroup = :#{#tutorialGroup}""") + SELECT session + FROM TutorialGroupSession session + WHERE session.start <= :#{#end} + AND session.end >= :#{#start} + AND session.tutorialGroupSchedule IS NULL + AND session.tutorialGroup = :#{#tutorialGroup} + """) Set findOverlappingIndividualSessionsInSameTutorialGroup(@Param("tutorialGroup") TutorialGroup tutorialGroup, @Param("start") ZonedDateTime start, @Param("end") ZonedDateTime end); @Query(""" - SELECT tutorialGroupSession - FROM TutorialGroupSession tutorialGroupSession - WHERE tutorialGroupSession.start <= :#{#end} AND tutorialGroupSession.end >= :#{#start} - AND tutorialGroupSession.tutorialGroup.course = :#{#course}""") + SELECT session + FROM TutorialGroupSession session + WHERE session.start <= :#{#end} + AND session.end >= :#{#start} + AND session.tutorialGroup.course = :#{#course} + """) Set findAllBetween(@Param("course") Course course, @Param("start") ZonedDateTime start, @Param("end") ZonedDateTime end); Set findAllByTutorialGroupFreePeriodId(Long freePeriodId); diff --git a/src/main/java/de/tum/in/www1/artemis/repository/tutorialgroups/TutorialGroupsConfigurationRepository.java b/src/main/java/de/tum/in/www1/artemis/repository/tutorialgroups/TutorialGroupsConfigurationRepository.java index 8665e4803d63..e3ff6dd4b360 100644 --- a/src/main/java/de/tum/in/www1/artemis/repository/tutorialgroups/TutorialGroupsConfigurationRepository.java +++ b/src/main/java/de/tum/in/www1/artemis/repository/tutorialgroups/TutorialGroupsConfigurationRepository.java @@ -14,9 +14,10 @@ public interface TutorialGroupsConfigurationRepository extends JpaRepository { @Query(""" - SELECT tutorialGroupConfiguration - FROM TutorialGroupsConfiguration tutorialGroupConfiguration LEFT JOIN tutorialGroupConfiguration.tutorialGroupFreePeriods - WHERE tutorialGroupConfiguration.id = :#{#tutorialGroupConfigurationId} + SELECT t + FROM TutorialGroupsConfiguration t + LEFT JOIN t.tutorialGroupFreePeriods + WHERE t.id = :#{#tutorialGroupConfigurationId} """) Optional findByIdWithEagerTutorialGroupFreePeriods(@Param("tutorialGroupConfigurationId") Long tutorialGroupConfigurationId); @@ -26,9 +27,10 @@ default TutorialGroupsConfiguration findByIdWithEagerTutorialGroupFreePeriodsEls } @Query(""" - SELECT tutorialGroupConfiguration - FROM TutorialGroupsConfiguration tutorialGroupConfiguration LEFT JOIN FETCH tutorialGroupConfiguration.tutorialGroupFreePeriods - WHERE tutorialGroupConfiguration.course.id = :#{#courseId} + SELECT t + FROM TutorialGroupsConfiguration t + LEFT JOIN FETCH t.tutorialGroupFreePeriods + WHERE t.course.id = :#{#courseId} """) Optional findByCourseIdWithEagerTutorialGroupFreePeriods(Long courseId); } diff --git a/src/main/java/de/tum/in/www1/artemis/service/CourseService.java b/src/main/java/de/tum/in/www1/artemis/service/CourseService.java index d2695e79969a..ddb69c0ce16c 100644 --- a/src/main/java/de/tum/in/www1/artemis/service/CourseService.java +++ b/src/main/java/de/tum/in/www1/artemis/service/CourseService.java @@ -267,7 +267,14 @@ public Course findOneWithExercisesAndLecturesAndExamsAndCompetenciesAndTutorialG course.setExercises(exerciseService.filterExercisesForCourse(course, user)); exerciseService.loadExerciseDetailsIfNecessary(course, user); course.setExams(examRepository.findByCourseIdsForUser(Set.of(course.getId()), user.getId(), user.getGroups(), ZonedDateTime.now())); + // TODO: in the future, we only want to know if lectures exist, the actual lectures will be loaded when the user navigates into the lecture course.setLectures(lectureService.filterVisibleLecturesWithActiveAttachments(course, course.getLectures(), user)); + // NOTE: in this call we only want to know if competencies exist in the course, we will load them when the user navigates into them + course.setNumberOfCompetencies(competencyRepository.countByCourse(course)); + // NOTE: in this call we only want to know if prerequisites exist in the course, we will load them when the user navigates into them + course.setNumberOfPrerequisites(competencyRepository.countPrerequisitesByCourseId(course.getId())); + // NOTE: in this call we only want to know if tutorial groups exist in the course, we will load them when the user navigates into them + course.setNumberOfTutorialGroups(tutorialGroupRepository.countByCourse(course)); if (authCheckService.isOnlyStudentInCourse(course, user)) { course.setExams(examRepository.filterVisibleExams(course.getExams())); } @@ -292,6 +299,7 @@ public Set findAllActiveForUser(User user) { */ public List findAllActiveWithExercisesAndLecturesAndExamsForUser(User user) { long start = System.nanoTime(); + // TODO: in the future we only need the number of lectures and no additional values var userVisibleCourses = courseRepository.findAllActiveWithLectures().stream().filter(course -> isCourseVisibleForUser(user, course)).toList(); if (log.isDebugEnabled()) { @@ -313,7 +321,11 @@ public List findAllActiveWithExercisesAndLecturesAndExamsForUser(User us course.setExercises(exerciseService.filterExercisesForCourse(course, user)); exerciseService.loadExerciseDetailsIfNecessary(course, user); course.setExams(allExams.stream().filter(ex -> ex.getCourse().getId().equals(course.getId())).collect(Collectors.toSet())); + // TODO: we only need the number of exams here course.setLectures(lectureService.filterVisibleLecturesWithActiveAttachments(course, course.getLectures(), user)); + course.setNumberOfLectures((long) course.getLectures().size()); + // we do not send them to the client, not needed + course.setLectures(Set.of()); }).toList(); if (log.isDebugEnabled()) { diff --git a/src/main/java/de/tum/in/www1/artemis/service/learningpath/LearningPathNgxService.java b/src/main/java/de/tum/in/www1/artemis/service/learningpath/LearningPathNgxService.java index f9697f4410d7..dd0997649584 100644 --- a/src/main/java/de/tum/in/www1/artemis/service/learningpath/LearningPathNgxService.java +++ b/src/main/java/de/tum/in/www1/artemis/service/learningpath/LearningPathNgxService.java @@ -126,7 +126,7 @@ private void generateNgxGraphRepresentationForCompetency(LearningPath learningPa * @param edges set of edges to store the new edges */ private void generateNgxGraphRepresentationForRelations(LearningPath learningPath, Set nodes, Set edges) { - final var relations = competencyRelationRepository.findAllByCourseId(learningPath.getCourse().getId()); + final var relations = competencyRelationRepository.findAllWithHeadAndTailByCourseId(learningPath.getCourse().getId()); // compute match clusters Map competencyToMatchCluster = new HashMap<>(); diff --git a/src/main/java/de/tum/in/www1/artemis/service/tutorialgroups/TutorialGroupService.java b/src/main/java/de/tum/in/www1/artemis/service/tutorialgroups/TutorialGroupService.java index 96005f6e147c..b4af88af4935 100644 --- a/src/main/java/de/tum/in/www1/artemis/service/tutorialgroups/TutorialGroupService.java +++ b/src/main/java/de/tum/in/www1/artemis/service/tutorialgroups/TutorialGroupService.java @@ -4,7 +4,6 @@ import static javax.persistence.Persistence.getPersistenceUtil; import java.time.ZonedDateTime; -import java.time.temporal.ChronoUnit; import java.util.*; import java.util.function.Function; import java.util.stream.Collectors; @@ -101,8 +100,9 @@ public void setTransientPropertiesForUser(User user, TutorialGroup tutorialGroup tutorialGroup.setTeachingAssistantName(null); } - var channel = tutorialGroupChannelManagementService.getTutorialGroupChannel(tutorialGroup); - channel.ifPresent(value -> tutorialGroup.setChannel(conversationDTOService.convertChannelToDto(user, value))); + if (tutorialGroup.getTutorialGroupChannel() != null) { + tutorialGroup.setChannel(conversationDTOService.convertChannelToDto(user, tutorialGroup.getTutorialGroupChannel())); + } this.setNextSession(tutorialGroup); this.setAverageAttendance(tutorialGroup); @@ -134,11 +134,7 @@ private void setAverageAttendance(TutorialGroup tutorialGroup) { && tutorialGroupSession.getEnd().isBefore(ZonedDateTime.now())) .sorted(Comparator.comparing(TutorialGroupSession::getStart).reversed()).limit(3) .map(tutorialGroupSession -> Optional.ofNullable(tutorialGroupSession.getAttendanceCount())).flatMap(Optional::stream).mapToInt(attendance -> attendance).average() - .ifPresentOrElse(value -> { - tutorialGroup.setAverageAttendance((int) Math.round(value)); - }, () -> { - tutorialGroup.setAverageAttendance(null); - }); + .ifPresentOrElse(value -> tutorialGroup.setAverageAttendance((int) Math.round(value)), () -> tutorialGroup.setAverageAttendance(null)); } /** @@ -153,11 +149,11 @@ private void setNextSession(TutorialGroup tutorialGroup) { // we show currently running sessions and up to 30 minutes after the end of the session so that students can still join and tutors can easily update the attendance of // the session nextSessionOptional = tutorialGroup.getTutorialGroupSessions().stream().filter(session -> session.getStatus() == TutorialGroupSessionStatus.ACTIVE) - .filter(session -> session.getEnd().plus(30, ChronoUnit.MINUTES).isAfter(ZonedDateTime.now())).min(Comparator.comparing(TutorialGroupSession::getStart)); + .filter(session -> session.getEnd().plusMinutes(30).isAfter(ZonedDateTime.now())).min(Comparator.comparing(TutorialGroupSession::getStart)); } else { var nextSessions = tutorialGroupSessionRepository.findNextSessionsOfStatus(tutorialGroup.getId(), ZonedDateTime.now(), TutorialGroupSessionStatus.ACTIVE); - if (nextSessions.size() > 0) { + if (!nextSessions.isEmpty()) { nextSessionOptional = Optional.of(nextSessions.get(0)); } } @@ -406,9 +402,7 @@ private Set findOrCreateTutorialGroups(Course course, Set(foundTutorialGroups); tutorialGroupsMentionedInRegistrations.addAll(tutorialGroupRepository.saveAll(tutorialGroupsToCreate)); - tutorialGroupsMentionedInRegistrations.forEach(tutorialGroup -> { - tutorialGroupChannelManagementService.createChannelForTutorialGroup(tutorialGroup); - }); + tutorialGroupsMentionedInRegistrations.forEach(tutorialGroupChannelManagementService::createChannelForTutorialGroup); return tutorialGroupsMentionedInRegistrations; } @@ -507,7 +501,8 @@ private Set findUsersByLogins(Set logins, String groupName) { */ public Set findAllForCourse(@NotNull Course course, @NotNull User user) { // do not load all sessions here as they are not needed for the overview page and would slow down the request - Set tutorialGroups = tutorialGroupRepository.findAllByCourseIdWithTeachingAssistantAndRegistrations(course.getId()); + Set tutorialGroups = tutorialGroupRepository.findAllByCourseIdWithTeachingAssistantRegistrationsAndSchedule(course.getId()); + // TODO: this is some overkill, we calculate way too many information with way too many database calls, we must reduce this tutorialGroups.forEach(tutorialGroup -> this.setTransientPropertiesForUser(user, tutorialGroup)); tutorialGroups.forEach(tutorialGroup -> { if (!this.isAllowedToSeePrivateTutorialGroupInformation(tutorialGroup, user)) { diff --git a/src/main/java/de/tum/in/www1/artemis/web/rest/CompetencyResource.java b/src/main/java/de/tum/in/www1/artemis/web/rest/CompetencyResource.java index 058fedc41e60..c7fb1de567f4 100644 --- a/src/main/java/de/tum/in/www1/artemis/web/rest/CompetencyResource.java +++ b/src/main/java/de/tum/in/www1/artemis/web/rest/CompetencyResource.java @@ -122,13 +122,21 @@ public ResponseEntity> getAllCompetenciesOnPage( */ @GetMapping("/courses/{courseId}/competencies") @EnforceAtLeastStudent - public ResponseEntity> getCompetencies(@PathVariable Long courseId) { + public ResponseEntity> getCompetenciesWithProgress(@PathVariable Long courseId) { log.debug("REST request to get competencies for course with id: {}", courseId); Course course = courseRepository.findByIdElseThrow(courseId); User user = userRepository.getUserWithGroupsAndAuthorities(); authorizationCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.STUDENT, course, user); + // First load competencies Set competencies = competencyRepository.findAllForCourse(course.getId()); + // Then load the progress of the user + Set progressForUser = competencyProgressRepository.findByCompetenciesAndUser(competencies, user.getId()); + // Map the progress into the competency now + competencies.forEach(competency -> { + var userProgress = progressForUser.stream().filter(progress -> progress.getCompetency().getId().equals(competency.getId())).findFirst().map(Set::of).orElseGet(Set::of); + competency.setUserProgress(userProgress); + }); return ResponseEntity.ok(new ArrayList<>(competencies)); } @@ -406,7 +414,7 @@ public ResponseEntity createCompetencyRelation(@PathVariable relation.setType(relationType); var competencies = competencyRepository.findAllForCourse(course.getId()); - var competencyRelations = competencyRelationRepository.findAllByCourseId(course.getId()); + var competencyRelations = competencyRelationRepository.findAllWithHeadAndTailByCourseId(course.getId()); competencyRelations.add(relation); if (competencyService.doesCreateCircularRelation(competencies, competencyRelations)) { throw new BadRequestException("You can't define circular dependencies between competencies"); diff --git a/src/main/java/de/tum/in/www1/artemis/web/rest/CourseResource.java b/src/main/java/de/tum/in/www1/artemis/web/rest/CourseResource.java index ebd9915ff5f3..aebcee6e2d5f 100644 --- a/src/main/java/de/tum/in/www1/artemis/web/rest/CourseResource.java +++ b/src/main/java/de/tum/in/www1/artemis/web/rest/CourseResource.java @@ -352,7 +352,7 @@ public ResponseEntity> unenrollFromCourse(@PathVariable Long courseI */ @GetMapping("courses") @EnforceAtLeastTutor - public List getAllCourses(@RequestParam(defaultValue = "false") boolean onlyActive) { + public List getCourses(@RequestParam(defaultValue = "false") boolean onlyActive) { 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 @@ -373,7 +373,7 @@ public List getAllCourses(@RequestParam(defaultValue = "false") boolean */ @GetMapping("courses/courses-with-quiz") @EnforceAtLeastEditor - public List getAllCoursesWithQuizExercises() { + public List getCoursesWithQuizExercises() { User user = userRepository.getUserWithGroupsAndAuthorities(); if (authCheckService.isAdmin(user)) { return courseRepository.findAllWithQuizExercisesWithEagerExercises(); @@ -392,9 +392,9 @@ public List getAllCoursesWithQuizExercises() { */ @GetMapping("courses/with-user-stats") @EnforceAtLeastTutor - public List getAllCoursesWithUserStats(@RequestParam(defaultValue = "false") boolean onlyActive) { + public List getCoursesWithUserStats(@RequestParam(defaultValue = "false") boolean onlyActive) { log.debug("get courses with user stats, only active: {}", onlyActive); - List courses = getAllCourses(onlyActive); + List courses = getCourses(onlyActive); for (Course course : courses) { course.setNumberOfInstructors(userRepository.countUserInGroup(course.getInstructorGroupName())); course.setNumberOfTeachingAssistants(userRepository.countUserInGroup(course.getTeachingAssistantGroupName())); @@ -412,7 +412,7 @@ public List getAllCoursesWithUserStats(@RequestParam(defaultValue = "fal */ @GetMapping("courses/course-management-overview") @EnforceAtLeastTutor - public List getAllCoursesForManagementOverview(@RequestParam(defaultValue = "false") boolean onlyActive) { + public List getCoursesForManagementOverview(@RequestParam(defaultValue = "false") boolean onlyActive) { return courseService.getAllCoursesForManagementOverview(onlyActive); } @@ -442,7 +442,7 @@ public ResponseEntity getCourseForEnrollment(@PathVariable long courseId */ @GetMapping("courses/for-enrollment") @EnforceAtLeastStudent - public List getAllCoursesForEnrollment() { + public List getCoursesForEnrollment() { log.debug("REST request to get all currently active courses that are not online courses"); User user = userRepository.getUserWithGroupsAndAuthoritiesAndOrganizations(); return courseService.findAllEnrollableForUser(user).stream().filter(course -> authCheckService.isUserAllowedToSelfEnrollInCourse(user, course)).toList(); @@ -464,6 +464,7 @@ public ResponseEntity getCourseForDashboard(@PathVariable User user = userRepository.getUserWithGroupsAndAuthorities(); Course course = courseService.findOneWithExercisesAndLecturesAndExamsAndCompetenciesAndTutorialGroupsForUser(courseId, user); + log.debug("courseService.findOneWithExercisesAndLecturesAndExamsAndCompetenciesAndTutorialGroupsForUser done"); if (!authCheckService.isAtLeastStudentInCourse(course, user)) { // user might be allowed to enroll in the course // We need the course with organizations so that we can check if the user is allowed to enroll @@ -481,9 +482,11 @@ public ResponseEntity getCourseForDashboard(@PathVariable } courseService.fetchParticipationsWithSubmissionsAndResultsForCourses(List.of(course), user, true); + log.debug("courseService.fetchParticipationsWithSubmissionsAndResultsForCourses done"); courseService.fetchPlagiarismCasesForCourseExercises(course.getExercises(), user.getId()); + log.debug("courseService.fetchPlagiarismCasesForCourseExercises done"); GradingScale gradingScale = gradingScaleRepository.findByCourseId(course.getId()).orElse(null); - + log.debug("gradingScaleRepository.findByCourseId done"); CourseForDashboardDTO courseForDashboardDTO = courseScoreCalculationService.getScoresAndParticipationResults(course, gradingScale, user.getId()); logDuration(List.of(course), user, timeNanoStart, "courses/" + courseId + "/for-dashboard (single course)"); return ResponseEntity.ok(courseForDashboardDTO); @@ -498,17 +501,21 @@ public ResponseEntity getCourseForDashboard(@PathVariable */ @GetMapping("courses/for-dashboard") @EnforceAtLeastStudent - public List getAllCoursesForDashboard() { + public List getCoursesForDashboard() { long timeNanoStart = System.nanoTime(); User user = userRepository.getUserWithGroupsAndAuthorities(); - log.debug( - "REST request to get all courses the user {} has access to with exams, lectures, exercises, participations, submissions and results + the calculated scores the user achieved in each of those courses", - user.getLogin()); + log.debug("Request to get all courses user {} has access to with exams, lectures, exercises, participations, submissions and results + calculated scores", user.getLogin()); List courses = courseService.findAllActiveWithExercisesAndLecturesAndExamsForUser(user); + log.debug("courseService.findAllActiveWithExercisesAndLecturesAndExamsForUser done"); courseService.fetchParticipationsWithSubmissionsAndResultsForCourses(courses, user, false); + log.debug("courseService.fetchParticipationsWithSubmissionsAndResultsForCourses done"); + // TODO: we should avoid fetching plagiarism in the future, it's unnecessary for 99.9% of the cases courseService.fetchPlagiarismCasesForCourseExercises(courses.stream().flatMap(course -> course.getExercises().stream()).collect(Collectors.toSet()), user.getId()); + log.debug("courseService.fetchPlagiarismCasesForCourseExercises done"); + // TODO: loading the grading scale here is only done to handle edge cases, we should avoid it, it's unnecessary for 90% of the cases and can be done when navigating into + // the course Set gradingScales = gradingScaleRepository.findAllByCourseIds(courses.stream().map(Course::getId).collect(Collectors.toSet())); - + log.debug("gradingScaleRepository.findAllByCourseIds done"); List coursesForDashboard = new ArrayList<>(); for (Course course : courses) { GradingScale gradingScale = gradingScales.stream().filter(scale -> scale.getCourse().getId().equals(course.getId())).findFirst().orElse(null); @@ -538,7 +545,7 @@ private void logDuration(List courses, User user, long timeNanoStart, St */ @GetMapping("courses/for-notifications") @EnforceAtLeastStudent - public Set getAllCoursesForNotifications() { + public Set getCoursesForNotifications() { log.debug("REST request to get all Courses the user has access to"); User user = userRepository.getUserWithGroupsAndAuthorities(); return courseService.findAllActiveForUser(user); @@ -824,7 +831,7 @@ public ResponseEntity> getCategoriesInCourse(@PathVariable Long cour */ @GetMapping("courses/{courseId}/students") @EnforceAtLeastInstructor - public ResponseEntity> getAllStudentsInCourse(@PathVariable Long courseId) { + public ResponseEntity> getStudentsInCourse(@PathVariable Long courseId) { log.debug("REST request to get all students in course : {}", courseId); Course course = courseRepository.findByIdElseThrow(courseId); return courseService.getAllUsersInGroup(course, course.getStudentGroupName()); @@ -908,7 +915,7 @@ public ResponseEntity> searchUsersInCourse(@PathVariable */ @GetMapping("courses/{courseId}/tutors") @EnforceAtLeastInstructor - public ResponseEntity> getAllTutorsInCourse(@PathVariable Long courseId) { + public ResponseEntity> getTutorsInCourse(@PathVariable Long courseId) { log.debug("REST request to get all tutors in course : {}", courseId); Course course = courseRepository.findByIdElseThrow(courseId); return courseService.getAllUsersInGroup(course, course.getTeachingAssistantGroupName()); @@ -922,7 +929,7 @@ public ResponseEntity> getAllTutorsInCourse(@PathVariable Long courseI */ @GetMapping("courses/{courseId}/editors") @EnforceAtLeastInstructor - public ResponseEntity> getAllEditorsInCourse(@PathVariable Long courseId) { + public ResponseEntity> getEditorsInCourse(@PathVariable Long courseId) { log.debug("REST request to get all editors in course : {}", courseId); Course course = courseRepository.findByIdElseThrow(courseId); return courseService.getAllUsersInGroup(course, course.getEditorGroupName()); @@ -936,7 +943,7 @@ public ResponseEntity> getAllEditorsInCourse(@PathVariable Long course */ @GetMapping("courses/{courseId}/instructors") @EnforceAtLeastInstructor - public ResponseEntity> getAllInstructorsInCourse(@PathVariable Long courseId) { + public ResponseEntity> getInstructorsInCourse(@PathVariable Long courseId) { log.debug("REST request to get all instructors in course : {}", courseId); Course course = courseRepository.findByIdElseThrow(courseId); return courseService.getAllUsersInGroup(course, course.getInstructorGroupName()); diff --git a/src/main/webapp/app/entities/course.model.ts b/src/main/webapp/app/entities/course.model.ts index 27f5dfd1c985..f60ea506cf3a 100644 --- a/src/main/webapp/app/entities/course.model.ts +++ b/src/main/webapp/app/entities/course.model.ts @@ -98,6 +98,12 @@ export class Course implements BaseEntity { public numberOfEditors?: number; public numberOfInstructors?: number; + // helper attributes to determine if certain tabs in the client are shown + public numberOfLectures?: number; + public numberOfTutorialGroups?: number; + public numberOfCompetencies?: number; + public numberOfPrerequisites?: number; + public exercises?: Exercise[]; public lectures?: Lecture[]; public competencies?: Competency[]; diff --git a/src/main/webapp/app/overview/course-card.component.ts b/src/main/webapp/app/overview/course-card.component.ts index 1ce9f04b8c07..48b9ce3c93c8 100644 --- a/src/main/webapp/app/overview/course-card.component.ts +++ b/src/main/webapp/app/overview/course-card.component.ts @@ -89,9 +89,7 @@ export class CourseCardComponent implements OnChanges { this.ngxDoughnutData = [...this.ngxDoughnutData]; } - if (this.course.lectures) { - this.lectureCount = this.course.lectures.length; - } + this.lectureCount = this.course.numberOfLectures ?? this.course.lectures?.length ?? 0; if (this.course.exams) { this.examCount = this.course.exams.length; diff --git a/src/main/webapp/app/overview/course-competencies/course-competencies.component.ts b/src/main/webapp/app/overview/course-competencies/course-competencies.component.ts index 0f6e523ec41b..ad43c290c0b5 100644 --- a/src/main/webapp/app/overview/course-competencies/course-competencies.component.ts +++ b/src/main/webapp/app/overview/course-competencies/course-competencies.component.ts @@ -5,7 +5,7 @@ import { AlertService } from 'app/core/util/alert.service'; import { onError } from 'app/shared/util/global.utils'; import { HttpErrorResponse } from '@angular/common/http'; import { Competency } from 'app/entities/competency.model'; -import { Subscription, forkJoin } from 'rxjs'; +import { forkJoin } from 'rxjs'; import { Course } from 'app/entities/course.model'; import { faAngleDown, faAngleUp } from '@fortawesome/free-solid-svg-icons'; import { CourseStorageService } from 'app/course/manage/course-storage.service'; @@ -28,8 +28,6 @@ export class CourseCompetenciesComponent implements OnInit { faAngleDown = faAngleDown; faAngleUp = faAngleUp; - private courseUpdateSubscription?: Subscription; - constructor( private activatedRoute: ActivatedRoute, private alertService: AlertService, @@ -43,14 +41,14 @@ export class CourseCompetenciesComponent implements OnInit { }); this.setCourse(this.courseStorageService.getCourse(this.courseId)); - this.courseUpdateSubscription = this.courseStorageService.subscribeToCourseUpdates(this.courseId).subscribe((course) => this.setCourse(course)); } private setCourse(course?: Course) { this.course = course; - if (this.course && this.course.competencies && this.course.prerequisites) { - this.competencies = this.course.competencies; - this.prerequisites = this.course.prerequisites; + // Note: this component is only shown if there are at least 1 competencies or at least 1 prerequisites, so if they do not exist, we load the data from the server + if (this.course && ((this.course.competencies && this.course.competencies.length > 0) || (this.course.prerequisites && this.course.prerequisites.length > 0))) { + this.competencies = this.course.competencies || []; + this.prerequisites = this.course.prerequisites || []; } else { this.loadData(); } @@ -61,9 +59,9 @@ export class CourseCompetenciesComponent implements OnInit { } get countMasteredCompetencies() { - return this.competencies.filter((lg) => { - if (lg.userProgress?.length && lg.masteryThreshold) { - return lg.userProgress.first()!.progress == 100 && lg.userProgress.first()!.confidence! >= lg.masteryThreshold!; + return this.competencies.filter((competency) => { + if (competency.userProgress?.length && competency.masteryThreshold) { + return competency.userProgress.first()!.progress == 100 && competency.userProgress.first()!.confidence! >= competency.masteryThreshold!; } return false; }).length; @@ -82,6 +80,11 @@ export class CourseCompetenciesComponent implements OnInit { next: ([competencies, prerequisites]) => { this.competencies = competencies.body!; this.prerequisites = prerequisites.body!; + // Also update the course, so we do not need to fetch again next time + if (this.course) { + this.course.competencies = this.competencies; + this.course.prerequisites = this.prerequisites; + } this.isLoading = false; }, error: (errorResponse: HttpErrorResponse) => onError(this.alertService, errorResponse), diff --git a/src/main/webapp/app/overview/course-overview.component.ts b/src/main/webapp/app/overview/course-overview.component.ts index 6b5212fc4c32..9be31b0d9db4 100644 --- a/src/main/webapp/app/overview/course-overview.component.ts +++ b/src/main/webapp/app/overview/course-overview.component.ts @@ -354,14 +354,14 @@ export class CourseOverviewComponent implements OnInit, OnDestroy, AfterViewInit * Check if the course has any competencies or prerequisites */ hasCompetencies(): boolean { - return !!(this.course?.competencies?.length || this.course?.prerequisites?.length); + return !!(this.course?.numberOfCompetencies || this.course?.numberOfPrerequisites); } /** * Check if the course has a tutorial groups */ hasTutorialGroups(): boolean { - return !!this.course?.tutorialGroups?.length; + return !!this.course?.numberOfTutorialGroups; } /** diff --git a/src/test/java/de/tum/in/www1/artemis/DatabaseQueryCountTest.java b/src/test/java/de/tum/in/www1/artemis/DatabaseQueryCountTest.java index bbf8666fcdfc..fc970c26bf18 100644 --- a/src/test/java/de/tum/in/www1/artemis/DatabaseQueryCountTest.java +++ b/src/test/java/de/tum/in/www1/artemis/DatabaseQueryCountTest.java @@ -73,8 +73,7 @@ void testGetAllCoursesForDashboardRealisticQueryCount() throws Exception { var userCourse = request.get("/api/courses/" + course.getId() + "/for-dashboard", HttpStatus.OK, Course.class); log.info("Finish courses for dashboard call for one course"); return userCourse; - }).hasBeenCalledAtMostTimes(11); - // TODO: update the following description, we only have 11 and not 15 calls + }).hasBeenCalledAtMostTimes(15); // 1 DB call to get the user from the DB // 2 DB calls to get the course with exercise, lectures, exams // 1 DB call to load all exercises diff --git a/src/test/javascript/spec/component/course/course-overview.component.spec.ts b/src/test/javascript/spec/component/course/course-overview.component.spec.ts index d6225f337bcf..0ac916ecc568 100644 --- a/src/test/javascript/spec/component/course/course-overview.component.spec.ts +++ b/src/test/javascript/spec/component/course/course-overview.component.spec.ts @@ -92,6 +92,9 @@ const course2: Course = { competencies: [new Competency()], tutorialGroups: [new TutorialGroup()], prerequisites: [new Competency()], + numberOfCompetencies: 1, + numberOfPrerequisites: 1, + numberOfTutorialGroups: 1, }; @Component({ @@ -114,7 +117,6 @@ describe('CourseOverviewComponent', () => { let courseService: CourseManagementService; let courseStorageService: CourseStorageService; let teamService: TeamService; - let competencyService: CompetencyService; let tutorialGroupsService: TutorialGroupsService; let tutorialGroupsConfigurationService: TutorialGroupsConfigurationService; let jhiWebsocketService: JhiWebsocketService; @@ -181,7 +183,6 @@ describe('CourseOverviewComponent', () => { courseService = TestBed.inject(CourseManagementService); courseStorageService = TestBed.inject(CourseStorageService); teamService = TestBed.inject(TeamService); - competencyService = TestBed.inject(CompetencyService); tutorialGroupsService = TestBed.inject(TutorialGroupsService); tutorialGroupsConfigurationService = TestBed.inject(TutorialGroupsConfigurationService); jhiWebsocketService = TestBed.inject(JhiWebsocketService); @@ -408,10 +409,6 @@ describe('CourseOverviewComponent', () => { it('should have competencies and tutorial groups', () => { const getCourseStub = jest.spyOn(courseStorageService, 'getCourse'); - const competenciesResponse: HttpResponse = new HttpResponse({ - body: [new Competency()], - status: 200, - }); const tutorialGroupsResponse: HttpResponse = new HttpResponse({ body: [new TutorialGroup()], status: 200, @@ -421,8 +418,6 @@ describe('CourseOverviewComponent', () => { status: 200, }); - jest.spyOn(competencyService, 'getAllPrerequisitesForCourse').mockReturnValue(of(competenciesResponse)); - jest.spyOn(competencyService, 'getAllForCourse').mockReturnValue(of(competenciesResponse)); jest.spyOn(tutorialGroupsService, 'getAllForCourse').mockReturnValue(of(tutorialGroupsResponse)); jest.spyOn(tutorialGroupsConfigurationService, 'getOneOfCourse').mockReturnValue(of(configurationResponse));