-
Notifications
You must be signed in to change notification settings - Fork 295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adaptive learning
: Add implementation for grade or bonus goal in learner profile
#9699
base: feature/adaptive-learning/learner-profile
Are you sure you want to change the base?
Conversation
…/adaptive-learning/learner-profile-graded-exercises
WalkthroughThis pull request introduces several new classes and modifications to existing ones, primarily focusing on the management of learner profiles and their relationships with courses in a learning management system. Key additions include the Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 28
🧹 Outside diff range and nitpick comments (32)
src/main/java/de/tum/cit/aet/artemis/atlas/repository/LearnerProfileRepository.java (3)
20-20
: Consider adding @query annotation for findByUser.For better control over the generated SQL and potential performance optimization, consider explicitly defining the query.
+ @Query("SELECT lp FROM LearnerProfile lp WHERE lp.user = :user") Optional<LearnerProfile> findByUser(User user);
26-28
: Improve @transactional documentation and consider adding @query.
- The comment "ok because of delete" is not informative. Consider removing it or providing better documentation.
- Consider adding an explicit @query for better performance and clarity.
- @Transactional // ok because of delete + @Transactional + @Query("DELETE FROM LearnerProfile lp WHERE lp.user = :user") @Modifying void deleteByUser(User user);
20-28
: Add JavaDoc documentation for repository methods.Consider adding documentation to describe the purpose, parameters, and return values of each method.
+ /** + * Find a learner profile by its associated user. + * + * @param user the user to search for + * @return an Optional containing the learner profile if found + */ Optional<LearnerProfile> findByUser(User user); + /** + * Find a learner profile by user, throwing an exception if not found. + * + * @param user the user to search for + * @return the learner profile + * @throws EntityNotFoundException if no profile exists for the user + */ default LearnerProfile findByUserElseThrow(User user) + /** + * Delete a learner profile associated with the given user. + * + * @param user the user whose profile should be deleted + */ void deleteByUser(User user);src/main/java/de/tum/cit/aet/artemis/atlas/service/profile/LearnerProfileService.java (2)
13-15
: Consider adding class-level documentation.While the class structure and annotations are correct, adding Javadoc would improve code maintainability by documenting the service's purpose and responsibilities.
Add documentation like:
+/** + * Service for managing learner profiles. + * Handles the creation and deletion of learner profiles and their associated course profiles. + */ @Profile(PROFILE_CORE) @Service public class LearnerProfileService {
17-24
: Consider transaction management for repository operations.While the constructor injection is correctly implemented, the service might benefit from transaction management for database operations.
Consider adding transaction management:
@Profile(PROFILE_CORE) @Service +@Transactional(readOnly = true) public class LearnerProfileService {
src/main/java/de/tum/cit/aet/artemis/atlas/repository/CourseLearnerProfileRepository.java (2)
21-27
: Improve the transaction annotation documentation.While the implementation is correct, consider replacing the comment "ok because of delete" with more informative documentation explaining why the transaction is necessary and safe in this context.
- @Transactional // ok because of delete + @Transactional // Required for modifying queries, safe here as it's a simple delete operation
33-39
: Maintain consistent documentation improvement.The implementation is correct, but for consistency, improve the transaction annotation documentation similar to the other methods.
- @Transactional // ok because of delete + @Transactional // Required for modifying queries, safe here as it's a simple delete operationsrc/main/java/de/tum/cit/aet/artemis/atlas/domain/profile/LearnerProfile.java (1)
22-28
: Consider adding an index for the user_id foreign key.While the relationships are well-defined with appropriate fetch strategies, adding an index on the
user_id
column would optimize queries, especially for user lookups.Add the following index to improve query performance:
@Table(name = "learner_profile", indexes = { @Index(name = "idx_learner_profile_user_id", columnList = "user_id") })src/main/java/de/tum/cit/aet/artemis/atlas/domain/profile/CourseLearnerProfile.java (4)
14-16
: Add class-level documentation.Consider adding Javadoc to describe the purpose of this entity, its relationships, and the meaning of its fields. This will improve code maintainability and help other developers understand the domain model better.
+/** + * Entity representing a learner's profile specific to a course. + * This entity captures course-specific learning preferences and goals, + * including grade/bonus targets, time investment, and repetition intensity. + */ @Entity @Table(name = "course_learner_profile") public class CourseLearnerProfile extends DomainObject {
18-24
: Enhance relationship definitions with additional constraints.Consider adding the following JPA annotations to better define the relationships:
@ManyToOne -@JoinColumn(name = "learner_profile_id") +@JoinColumn(name = "learner_profile_id", nullable = false, + foreignKey = @ForeignKey(name = "fk_course_learner_profile_learner")) private LearnerProfile learnerProfile; @OneToOne(fetch = FetchType.LAZY) -@JoinColumn(name = "course_id") +@JoinColumn(name = "course_id", nullable = false, unique = true, + foreignKey = @ForeignKey(name = "fk_course_learner_profile_course")) private Course course;These changes will:
- Enforce referential integrity with proper foreign key constraints
- Ensure required relationships are not null
- Prevent duplicate course assignments
35-73
: Add parameter validation to setters.Consider adding validation to prevent invalid values:
+import java.util.Objects; public void setLearnerProfile(LearnerProfile learnerProfile) { + Objects.requireNonNull(learnerProfile, "learnerProfile must not be null"); this.learnerProfile = learnerProfile; } public void setCourse(Course course) { + Objects.requireNonNull(course, "course must not be null"); this.course = course; } public void setAimForGradeOrBonus(int aimForGradeOrBonus) { + if (aimForGradeOrBonus < 0 || aimForGradeOrBonus > 100) { + throw new IllegalArgumentException("aimForGradeOrBonus must be between 0 and 100"); + } this.aimForGradeOrBonus = aimForGradeOrBonus; } public void setTimeInvestment(int timeInvestment) { + if (timeInvestment < 0 || timeInvestment > 168) { + throw new IllegalArgumentException("timeInvestment must be between 0 and 168 hours"); + } this.timeInvestment = timeInvestment; } public void setRepetitionIntensity(int repetitionIntensity) { + if (repetitionIntensity < 1 || repetitionIntensity > 5) { + throw new IllegalArgumentException("repetitionIntensity must be between 1 and 5"); + } this.repetitionIntensity = repetitionIntensity; }
14-16
: Add database indexes for better query performance.Consider adding indexes to improve query performance:
@Entity -@Table(name = "course_learner_profile") +@Table(name = "course_learner_profile", + indexes = { + @Index(name = "idx_course_learner_profile_learner", + columnList = "learner_profile_id"), + @Index(name = "idx_course_learner_profile_course", + columnList = "course_id") + }) public class CourseLearnerProfile extends DomainObject {src/main/java/de/tum/cit/aet/artemis/core/service/user/UserCreationService.java (1)
Line range hint
1-379
: Consider splitting user creation and profile management responsibilities.The
UserCreationService
is taking on additional responsibilities with profile management. This could lead to maintenance issues as the profile feature grows.Consider creating a separate
UserProfileOrchestrationService
to handle the coordination between user creation and profile management, following the Single Responsibility Principle.src/main/java/de/tum/cit/aet/artemis/core/domain/User.java (2)
229-231
: Consider marking field as @nullable and reviewing cascade options.The relationship definition looks good overall, but consider these improvements:
- Add
@Nullable
annotation to thelearnerProfile
field since a user might not have a profile initially- Review if
CascadeType.ALL
is necessary - consider using more specific cascade types (e.g.,PERSIST
,MERGE
) if not all operations need to cascade@OneToOne(mappedBy = "user", fetch = FetchType.LAZY, cascade = CascadeType.ALL, orphanRemoval = true) @JsonIgnoreProperties("user") - private LearnerProfile learnerProfile; + @Nullable + private LearnerProfile learnerProfile;
576-582
: Add documentation and null-safety annotations to getter/setter methods.The methods should include:
- JavaDoc documentation explaining the purpose and behavior
- Null-safety annotations for better API understanding
- Consider adding validation in the setter method
+ /** + * Gets the learner profile associated with this user. + * @return the learner profile, or null if not set + */ + @Nullable public LearnerProfile getLearnerProfile() { return learnerProfile; } + /** + * Sets the learner profile for this user. + * @param learnerProfile the learner profile to set + */ public void setLearnerProfile(@Nullable LearnerProfile learnerProfile) { this.learnerProfile = learnerProfile; }src/main/java/de/tum/cit/aet/artemis/atlas/web/LearningPathResource.java (3)
Line range hint
344-350
: Consider extracting the transformation logic for better readability.While the implementation is correct, the stream operations could be more readable by extracting the transformation logic into a separate method.
Consider refactoring like this:
- List<CompetencyNameDTO> competencyNames = recommendationState.recommendedOrderOfCompetencies().stream() - .map(competencyId -> recommendationState.competencyIdMap().get(competencyId)).map(CompetencyNameDTO::of).toList(); + List<CompetencyNameDTO> competencyNames = recommendationState.recommendedOrderOfCompetencies().stream() + .map(this::mapToCompetencyNameDTO) + .toList(); + } + + private CompetencyNameDTO mapToCompetencyNameDTO(Long competencyId) { + return CompetencyNameDTO.of(recommendationState.competencyIdMap().get(competencyId));
367-367
: Add error handling for course ID retrieval.While the implementation correctly retrieves the user with learner profile, it should handle potential null course scenarios.
Consider adding null checks:
- final var user = userRepository.getUserWithGroupsAndAuthoritiesAndLearnerProfile(learningPath.getCourse().getId()); + final var course = learningPath.getCourse(); + if (course == null) { + throw new IllegalStateException("Learning path must be associated with a course"); + } + final var user = userRepository.getUserWithGroupsAndAuthoritiesAndLearnerProfile(course.getId());
Line range hint
235-367
: Consider documenting the learner profile integration pattern.The integration of learner profiles across multiple endpoints follows a consistent pattern. Consider documenting this pattern in the class-level JavaDoc to help future maintainers understand the design decision.
src/main/java/de/tum/cit/aet/artemis/atlas/service/learningpath/LearningPathService.java (2)
138-143
: Improve method documentation for the new overloaded method.The JavaDoc comment is identical to the original method and doesn't describe the additional
students
parameter.Update the documentation:
/** - * Generate learning paths for all students enrolled in the course + * Generate learning paths for a specific set of students in the course * * @param course course the learning paths are created for + * @param students set of students for whom learning paths should be generated */ public void generateLearningPaths(@NotNull Course course, Set<User> students) {
419-420
: Consider optimizing data fetching strategy.The method
findWithCompetenciesAndReleasedLearningObjectsAndCompletedUsersAndLearnerProfileById
performs multiple separate queries to fetch related data. This could lead to N+1 query issues with large datasets.Consider:
- Using batch fetching for competencies and their related entities
- Implementing a custom query using SQL joins to fetch all required data in a single query
- Adding appropriate database indexes for frequently accessed columns
Example optimization for competency progress fetching:
- Map<Long, CompetencyProgress> competencyProgresses = competencyProgressRepository - .findAllByCompetencyIdsAndUserId(competencyIds, userId).stream() - .collect(Collectors.toMap(progress -> progress.getCompetency().getId(), cp -> cp)); + @Query(""" + SELECT cp FROM CompetencyProgress cp + JOIN FETCH cp.competency c + WHERE c.id IN :competencyIds AND cp.user.id = :userId + """) + List<CompetencyProgress> findAllByCompetencyIdsAndUserIdWithFetch( + @Param("competencyIds") Set<Long> competencyIds, + @Param("userId") Long userId);src/test/java/de/tum/cit/aet/artemis/atlas/learningpath/LearningPathIntegrationTest.java (3)
Line range hint
447-476
: Add documentation for test utility methods.The utility methods
createAndLinkTextExercise
andcreateAndLinkTextUnit
would benefit from JavaDoc comments explaining their purpose, parameters, and return values.Apply this documentation:
+ /** + * Creates a text exercise and links it to the given competency. + * @param competency The competency to link the exercise to + * @param withAssessment If true, creates a student score of 100% + * @return The created and linked text exercise + */ private TextExercise createAndLinkTextExercise(Competency competency, boolean withAssessment) { + /** + * Creates a text unit, adds it to the lecture, and links it to the given competency. + * @param student The student to set completion status for + * @param competency The competency to link the unit to + * @param completed If true, marks the unit as completed for the student + * @return The created and linked text unit + */ private TextUnit createAndLinkTextUnit(User student, Competency competency, boolean completed) {
Line range hint
35-76
: Consider extracting test data constants to a dedicated class.The test class contains numerous test data constants. Consider extracting these to a dedicated test data configuration class to improve maintainability and reusability.
Example refactor:
public class LearningPathTestData { public static final String TEST_PREFIX = "learningpathintegration"; public static final int NUMBER_OF_STUDENTS = 5; public static final String STUDENT1_OF_COURSE = TEST_PREFIX + "student1"; // ... other constants }
Line range hint
324-325
: Address TODO comment about duplicate learning objects.The TODO comment indicates a known issue where learning objects connected to multiple competencies appear multiple times in the learning path. This should be tracked and addressed.
Would you like me to create a GitHub issue to track this improvement task?
src/test/java/de/tum/cit/aet/artemis/programming/CourseGitlabJenkinsIntegrationTest.java (1)
705-705
: Consider improving test method organization.While the addition of learner profile creation is necessary for the new functionality, consider the following improvements:
- Rename the test method to reflect that it's testing both learner profile setup and course user management, e.g.,
testAddStudentOrTutorOrInstructorToCourseWithLearnerProfile
.- Consider splitting this into two separate test methods:
- One focusing on learner profile setup
- Another focusing on course user management with the learner profile as a prerequisite
This would improve test clarity and maintain the single responsibility principle for tests.
src/main/java/de/tum/cit/aet/artemis/core/repository/UserRepository.java (2)
102-112
: Consider adding an index on the login column for better query performance.The query looks well-structured and uses proper JOIN FETCH statements to avoid N+1 query problems. However, since this query filters by login, consider adding an index on the login column if not already present.
916-926
: Consider enhancing the error message for better debugging.The implementation is solid, but consider enhancing the error message in
getValueElseThrow
to include both the login and courseId for better debugging when the user is not found.- return getValueElseThrow(findOneWithGroupsAndAuthoritiesAndLearnerProfileByLogin(currentUserLogin, courseId)); + return getValueElseThrow( + findOneWithGroupsAndAuthoritiesAndLearnerProfileByLogin(currentUserLogin, courseId), + String.format("User not found with login '%s' and courseId '%d'", currentUserLogin, courseId) + );src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java (1)
1276-1283
: Improve null safety in addUserToCourseGroup methodThe method could benefit from additional null checks and validation.
Consider adding parameter validation:
@NotNull public ResponseEntity<Void> addUserToCourseGroup(String userLogin, User instructorOrAdmin, Course course, String group) { + if (userLogin == null || instructorOrAdmin == null || course == null || group == null) { + throw new IllegalArgumentException("Required parameters cannot be null"); + } + if (authCheckService.isAtLeastInstructorInCourse(course, instructorOrAdmin)) { Optional<User> userToAddToGroup = userRepository.findOneWithGroupsAndAuthoritiesByLogin(userLogin); if (userToAddToGroup.isEmpty()) { throw new EntityNotFoundException("User", userLogin); } User user = userToAddToGroup.get(); courseService.addUserToGroup(user, group, course); return ResponseEntity.ok().body(null); } else { throw new AccessForbiddenException(); } }src/main/java/de/tum/cit/aet/artemis/atlas/service/learningpath/LearningPathRecommendationService.java (1)
604-615
: Clarify comparator behavior for low preferenceIn
getExerciseOrderComparator
, whenaimForGradeOrBonus
is low, the comparator returnsComparator.comparing(ignored -> 0)
, which maintains the original order. Consider adding a comment to explain this choice for better code clarity.src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java (2)
1056-1060
: Optimize database call by reusing existingcourse
objectIn
addUserToGroup
, you fetchcourseWithCompetencies
from the database, which may be unnecessary if the currentcourse
object already contains the required competencies and prerequisites. This additional call can affect performance.Consider reusing the existing
course
object or updating it to include the needed data to eliminate the extra database query.
512-512
: Consider checking if learning paths are enabled before deleting course learner profilesDeleting all course learner profiles when learning paths are disabled may be unnecessary and could introduce overhead.
Optionally, add a condition:
+if (course.getLearningPathsEnabled()) { courseLearnerProfileService.deleteAllForCourse(course); +}src/main/java/de/tum/cit/aet/artemis/atlas/repository/LearningPathRepository.java (2)
66-79
: Optimize query to improve performance due to multipleLEFT JOIN FETCH
clausesUsing multiple
LEFT JOIN FETCH
clauses can lead to performance issues by generating large result sets and causing Cartesian product effects. Consider the following options to optimize the query:
- Use
@EntityGraph
Annotation: Replace the JPQL query with an@EntityGraph
to specify the fetching strategy declaratively.- Fetch Associations Lazily: Modify the entity mappings to use lazy fetching where appropriate and load associations on demand.
- Split the Query: Break down the query into multiple simpler queries to reduce complexity and improve performance.
66-82
: Consider using@EntityGraph
for consistency and maintainabilityOther repository methods utilize the
@EntityGraph
annotation to manage fetching strategies. For consistency and easier maintenance, consider converting the JPQL query into a method annotated with@EntityGraph
.Example:
@EntityGraph(type = LOAD, attributePaths = { "competencies", "competencies.lectureUnitLinks.lectureUnit", "competencies.exerciseLinks.exercise", "user", "user.learnerProfile", "user.learnerProfile.courseLearnerProfiles" }) Optional<LearningPath> findWithCompetenciesAndLectureUnitsAndExercisesAndLearnerProfileById(long learningPathId);Ensure that the entity paths match the desired associations to be fetched.
🛑 Comments failed to post (28)
src/main/java/de/tum/cit/aet/artemis/atlas/repository/LearnerProfileRepository.java (1)
22-24: 🛠️ Refactor suggestion
Enhance error handling in findByUserElseThrow.
The method should provide a meaningful exception message when no profile is found.
default LearnerProfile findByUserElseThrow(User user) { - return getValueElseThrow(findByUser(user)); + return findByUser(user) + .orElseThrow(() -> new EntityNotFoundException( + String.format("LearnerProfile not found for user: %s", user.getLogin()) + )); }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.default LearnerProfile findByUserElseThrow(User user) { return findByUser(user) .orElseThrow(() -> new EntityNotFoundException( String.format("LearnerProfile not found for user: %s", user.getLogin()) )); }
src/main/java/de/tum/cit/aet/artemis/atlas/service/profile/LearnerProfileService.java (2)
26-30: 🛠️ Refactor suggestion
Enhance createProfile method with validation and error handling.
The method should validate input, handle potential errors, and include logging for better observability.
Consider these improvements:
+ @Transactional public LearnerProfile createProfile(User user) { + if (user == null) { + throw new IllegalArgumentException("User cannot be null"); + } + if (learnerProfileRepository.existsByUser(user)) { + throw new IllegalStateException("Profile already exists for user: " + user.getId()); + } var profile = new LearnerProfile(); profile.setUser(user); - return learnerProfileRepository.save(profile); + var savedProfile = learnerProfileRepository.save(profile); + log.debug("Created learner profile for user: {}", user.getId()); + return savedProfile; }Committable suggestion skipped: line range outside the PR's diff.
32-35:
⚠️ Potential issueImprove deleteProfile method with proper transaction management and error handling.
The current implementation might face referential integrity issues and lacks proper error handling.
Consider these improvements:
+ @Transactional public void deleteProfile(User user) { + if (user == null) { + throw new IllegalArgumentException("User cannot be null"); + } + var profile = learnerProfileRepository.findByUser(user) + .orElseThrow(() -> new IllegalStateException("Profile not found for user: " + user.getId())); + + log.debug("Deleting course profiles for user: {}", user.getId()); courseLearnerProfileRepository.deleteAllByUser(user); + + log.debug("Deleting learner profile for user: {}", user.getId()); learnerProfileRepository.deleteByUser(user); }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.@Transactional public void deleteProfile(User user) { if (user == null) { throw new IllegalArgumentException("User cannot be null"); } var profile = learnerProfileRepository.findByUser(user) .orElseThrow(() -> new IllegalStateException("Profile not found for user: " + user.getId())); log.debug("Deleting course profiles for user: {}", user.getId()); courseLearnerProfileRepository.deleteAllByUser(user); log.debug("Deleting learner profile for user: {}", user.getId()); learnerProfileRepository.deleteByUser(user); }
src/main/java/de/tum/cit/aet/artemis/atlas/repository/CourseLearnerProfileRepository.java (1)
29-31:
⚠️ Potential issueFix parameter name typo and improve documentation.
There are two issues to address:
- The parameter name contains a typo: "couese" should be "course"
- The transaction comment could be more informative
- @Transactional // ok because of delete - void deleteAllByCourse(Course couese); + @Transactional // Required for modifying queries, safe here as it's a simple delete operation + void deleteAllByCourse(Course course);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.@Transactional // Required for modifying queries, safe here as it's a simple delete operation @Modifying void deleteAllByCourse(Course course);
src/main/java/de/tum/cit/aet/artemis/atlas/domain/profile/LearnerProfile.java (2)
29-35: 🛠️ Refactor suggestion
Add bidirectional relationship management.
The User relationship methods should maintain both sides of the relationship to prevent inconsistencies.
Consider this implementation:
public void setUser(User user) { + if (this.user != null) { + this.user.setLearnerProfile(null); + } this.user = user; + if (user != null) { + user.setLearnerProfile(this); + } }Committable suggestion skipped: line range outside the PR's diff.
37-55: 🛠️ Refactor suggestion
Enhance collection management with defensive programming.
The collection management methods should:
- Include null checks
- Maintain bidirectional relationships
- Use defensive copying in getters
Consider these improvements:
public Set<CourseLearnerProfile> getCourseLearnerProfiles() { - return this.courseLearnerProfiles; + return new HashSet<>(this.courseLearnerProfiles); } public boolean addCourseLearnerProfile(CourseLearnerProfile courseLearnerProfile) { + if (courseLearnerProfile == null) { + return false; + } + courseLearnerProfile.setLearnerProfile(this); return this.courseLearnerProfiles.add(courseLearnerProfile); } public boolean removeCourseLearnerProfile(CourseLearnerProfile courseLearnerProfile) { + if (courseLearnerProfile == null) { + return false; + } + courseLearnerProfile.setLearnerProfile(null); return this.courseLearnerProfiles.remove(courseLearnerProfile); }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.public void setCourseLearnerProfiles(Set<CourseLearnerProfile> courseLearnerProfiles) { this.courseLearnerProfiles = courseLearnerProfiles; } public Set<CourseLearnerProfile> getCourseLearnerProfiles() { return new HashSet<>(this.courseLearnerProfiles); } public boolean addCourseLearnerProfile(CourseLearnerProfile courseLearnerProfile) { if (courseLearnerProfile == null) { return false; } courseLearnerProfile.setLearnerProfile(this); return this.courseLearnerProfiles.add(courseLearnerProfile); } public boolean addAllCourseLearnerProfiles(Collection<? extends CourseLearnerProfile> courseLearnerProfiles) { return this.courseLearnerProfiles.addAll(courseLearnerProfiles); } public boolean removeCourseLearnerProfile(CourseLearnerProfile courseLearnerProfile) { if (courseLearnerProfile == null) { return false; } courseLearnerProfile.setLearnerProfile(null); return this.courseLearnerProfiles.remove(courseLearnerProfile); }
src/main/java/de/tum/cit/aet/artemis/atlas/domain/profile/CourseLearnerProfile.java (1)
26-33: 🛠️ Refactor suggestion
Add field validation and documentation.
The integer fields need validation and documentation to ensure data integrity:
+import jakarta.validation.constraints.Min; +import jakarta.validation.constraints.Max; + /** + * The target grade or bonus points the learner aims to achieve. + * Valid range: 0-100 (percentage) + */ @Column(name = "aim_for_grade_or_bonus") + @Min(0) @Max(100) private int aimForGradeOrBonus; + /** + * Weekly time investment in hours. + * Valid range: 0-168 (max hours per week) + */ @Column(name = "time_investment") + @Min(0) @Max(168) private int timeInvestment; + /** + * Repetition intensity on a scale of 1-5. + * 1: minimal, 5: intensive + */ @Column(name = "repetition_intensity") + @Min(1) @Max(5) private int repetitionIntensity;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.import jakarta.validation.constraints.Min; import jakarta.validation.constraints.Max; /** * The target grade or bonus points the learner aims to achieve. * Valid range: 0-100 (percentage) */ @Column(name = "aim_for_grade_or_bonus") @Min(0) @Max(100) private int aimForGradeOrBonus; /** * Weekly time investment in hours. * Valid range: 0-168 (max hours per week) */ @Column(name = "time_investment") @Min(0) @Max(168) private int timeInvestment; /** * Repetition intensity on a scale of 1-5. * 1: minimal, 5: intensive */ @Column(name = "repetition_intensity") @Min(1) @Max(5) private int repetitionIntensity;
src/main/java/de/tum/cit/aet/artemis/atlas/service/profile/CourseLearnerProfileService.java (7)
25-29:
⚠️ Potential issueRemove unused constructor parameter
learnerProfileService
.The constructor accepts
LearnerProfileService
but never uses it. This violates the YAGNI principle.- public CourseLearnerProfileService(CourseLearnerProfileRepository courseLearnerProfileRepository, LearnerProfileService learnerProfileService, - LearnerProfileRepository learnerProfileRepository) { + public CourseLearnerProfileService(CourseLearnerProfileRepository courseLearnerProfileRepository, + LearnerProfileRepository learnerProfileRepository) { this.courseLearnerProfileRepository = courseLearnerProfileRepository; this.learnerProfileRepository = learnerProfileRepository; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.public CourseLearnerProfileService(CourseLearnerProfileRepository courseLearnerProfileRepository, LearnerProfileRepository learnerProfileRepository) { this.courseLearnerProfileRepository = courseLearnerProfileRepository; this.learnerProfileRepository = learnerProfileRepository; }
17-19: 🛠️ Refactor suggestion
Add class-level documentation and @transactional.
The service class should have:
- JavaDoc explaining its purpose and responsibilities
- Transaction management for database operations
@Profile(PROFILE_CORE) @Service +@Transactional +/** + * Service for managing course learner profiles. + * Handles the creation, deletion and management of profiles that track a learner's progress within a specific course. + */ public class CourseLearnerProfileService {Committable suggestion skipped: line range outside the PR's diff.
57-59: 🛠️ Refactor suggestion
Add validation and consider batch deletion for large datasets.
The bulk delete method needs improvements:
- Add input validation
- Add transaction boundary
- Consider batch deletion for large datasets
+ /** + * Deletes all course learner profiles for a given course. + * + * @param course the course, must not be null + * @throws IllegalArgumentException if course is null + */ + @Transactional public void deleteAllForCourse(Course course) { + if (course == null) { + throw new IllegalArgumentException("Course must not be null"); + } + // TODO: Consider implementing batch deletion for large datasets courseLearnerProfileRepository.deleteAllByCourse(course); }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements./** * Deletes all course learner profiles for a given course. * * @param course the course, must not be null * @throws IllegalArgumentException if course is null */ @Transactional public void deleteAllForCourse(Course course) { if (course == null) { throw new IllegalArgumentException("Course must not be null"); } // TODO: Consider implementing batch deletion for large datasets courseLearnerProfileRepository.deleteAllByCourse(course); }
41-51:
⚠️ Potential issueFix N+1 query issue and add validation.
The current implementation has several issues:
- Potential N+1 query problem when accessing learner profiles
- Missing input validation
- Direct entity access instead of using repository
- No duplicate handling
+ /** + * Bulk creates course learner profiles for multiple users. + * + * @param course the course, must not be null + * @param users the set of users, must not be null or empty + * @throws IllegalArgumentException if course or users is null/empty + */ + @Transactional public void createCourseLearnerProfiles(Course course, Set<User> users) { + if (course == null || users == null || users.isEmpty()) { + throw new IllegalArgumentException("Course and users must not be null/empty"); + } + + // Fetch all learner profiles in one query + var learnerProfiles = learnerProfileRepository.findAllByUserIn(users); + var userProfileMap = learnerProfiles.stream() + .collect(Collectors.toMap(profile -> profile.getUser().getId(), profile -> profile)); + Set<CourseLearnerProfile> courseProfiles = users.stream().map(user -> { + if (!userProfileMap.containsKey(user.getId())) { + throw new IllegalStateException("Learner profile not found for user: " + user.getId()); + } var courseProfile = new CourseLearnerProfile(); courseProfile.setCourse(course); - courseProfile.setLearnerProfile(user.getLearnerProfile()); + courseProfile.setLearnerProfile(userProfileMap.get(user.getId()));Committable suggestion skipped: line range outside the PR's diff.
31-39:
⚠️ Potential issueAdd input validation and duplicate handling.
The method needs several improvements:
- Validate input parameters
- Handle potential duplicate profiles
- Add method-level transaction annotation
+ /** + * Creates a course learner profile for a given course and user. + * + * @param course the course, must not be null + * @param user the user, must not be null + * @throws IllegalArgumentException if course or user is null + */ + @Transactional public void createCourseLearnerProfile(Course course, User user) { + if (course == null || user == null) { + throw new IllegalArgumentException("Course and user must not be null"); + } + + if (courseLearnerProfileRepository.existsByCourseAndUser(course, user)) { + throw new IllegalStateException("Course learner profile already exists"); + } + var courseProfile = new CourseLearnerProfile(); courseProfile.setCourse(course);Committable suggestion skipped: line range outside the PR's diff.
53-55: 🛠️ Refactor suggestion
Add input validation and transaction boundary.
The delete method needs input validation and transaction management.
+ /** + * Deletes a course learner profile for a given course and user. + * + * @param course the course, must not be null + * @param user the user, must not be null + * @throws IllegalArgumentException if course or user is null + */ + @Transactional public void deleteCourseLearnerProfile(Course course, User user) { + if (course == null || user == null) { + throw new IllegalArgumentException("Course and user must not be null"); + } courseLearnerProfileRepository.deleteByCourseAndUser(course, user); }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements./** * Deletes a course learner profile for a given course and user. * * @param course the course, must not be null * @param user the user, must not be null * @throws IllegalArgumentException if course or user is null */ @Transactional public void deleteCourseLearnerProfile(Course course, User user) { if (course == null || user == null) { throw new IllegalArgumentException("Course and user must not be null"); } courseLearnerProfileRepository.deleteByCourseAndUser(course, user); }
21-23: 💡 Codebase verification
Based on the verification results, I can now provide the final response:
Repository methods need to be added
The following required methods are missing from the repository interfaces:
findAllByUserIn
inLearnerProfileRepository
existsByCourseAndUser
inCourseLearnerProfileRepository
🔗 Analysis chain
Verify repository method declarations.
Let's verify that all required repository methods are properly declared in the interfaces.
Let me try a different approach to verify the repository interfaces using
rg
instead ofast-grep
.
Let me check for one more method that wasn't visible in the first 10 lines.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for repository interface declarations echo "Searching for repository interfaces and their method declarations..." # Check CourseLearnerProfileRepository echo "Checking CourseLearnerProfileRepository..." ast-grep --pattern 'interface CourseLearnerProfileRepository { $$$ deleteByCourseAndUser($$$); $$$ deleteAllByCourse($$$); $$$ existsByCourseAndUser($$$); $$$ }' # Check LearnerProfileRepository echo "Checking LearnerProfileRepository..." ast-grep --pattern 'interface LearnerProfileRepository { $$$ findByUserElseThrow($$$); $$$ findAllByUserIn($$$); $$$ }'Length of output: 882
Script:
#!/bin/bash # Search for repository interface declarations and their methods echo "Searching for CourseLearnerProfileRepository..." rg -A 10 "interface CourseLearnerProfileRepository" --type java echo -e "\nSearching for LearnerProfileRepository..." rg -A 10 "interface LearnerProfileRepository" --type javaLength of output: 3067
Script:
#!/bin/bash # Search for specific methods in repositories echo "Searching for findAllByUserIn in LearnerProfileRepository..." rg "findAllByUserIn" --type java echo -e "\nSearching for deleteAllByCourse and existsByCourseAndUser in CourseLearnerProfileRepository..." rg "deleteAllByCourse|existsByCourseAndUser" --type javaLength of output: 1552
src/test/java/de/tum/cit/aet/artemis/core/test_repository/UserTestRepository.java (1)
79-84: 🛠️ Refactor suggestion
Consider adding pagination and case sensitivity control.
The new query method has potential performance implications:
- Returns unbounded results without pagination
- Doesn't specify case sensitivity for the LIKE operation
Consider this alternative implementation:
- @Query(""" - SELECT user - FROM User user - WHERE user.login LIKE CONCAT(:userPrefix, '%') - """) - Set<User> findAllByUserPrefix(String userPrefix); + @Query(""" + SELECT user + FROM User user + WHERE LOWER(user.login) LIKE LOWER(CONCAT(:userPrefix, '%')) + """) + Page<User> findAllByUserPrefix(String userPrefix, Pageable pageable);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.@Query(""" SELECT user FROM User user WHERE LOWER(user.login) LIKE LOWER(CONCAT(:userPrefix, '%')) """) Page<User> findAllByUserPrefix(String userPrefix, Pageable pageable);
src/main/java/de/tum/cit/aet/artemis/core/service/UserScheduleService.java (1)
119-123: 🛠️ Refactor suggestion
Consider improving error handling and transaction management.
The current implementation has potential issues:
- If profile deletion fails after the user is deleted and flushed, it could leave orphaned profiles
- The operations are not wrapped in a transaction, risking partial completion
Consider refactoring to:
- Add error handling
- Ensure atomic operations
- Delete profile before flushing changes
private void deleteUser(User user) { + try { userRepository.delete(user); clearUserCaches(user); + learnerProfileService.deleteProfile(user); userRepository.flush(); - learnerProfileService.deleteProfile(user); + } catch (Exception e) { + log.error("Failed to delete user and associated profile", e); + throw new RuntimeException("User deletion failed", e); + } }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.try { userRepository.delete(user); clearUserCaches(user); learnerProfileService.deleteProfile(user); userRepository.flush(); } catch (Exception e) { log.error("Failed to delete user and associated profile", e); throw new RuntimeException("User deletion failed", e); } }
src/main/java/de/tum/cit/aet/artemis/core/service/user/UserCreationService.java (2)
197-204:
⚠️ Potential issueFix inconsistent return value and consider method refactoring.
The method has several issues:
- Returns
user
instead ofsavedUser
, potentially losing database-generated values- Method is growing large and handling multiple responsibilities
Fix the return value:
- return user; + return savedUser;Consider extracting the post-save operations into a separate method to improve readability and maintainability:
private User handlePostSaveOperations(User savedUser, String password, Set<String> groups) { optionalVcsUserManagementService.ifPresent(service -> service.createVcsUser(savedUser, password)); optionalCIUserManagementService.ifPresent(service -> service.createUser(savedUser, password)); addUserToGroupsInternal(savedUser, groups); learnerProfileService.createProfile(savedUser); return savedUser; }
149-150: 🛠️ Refactor suggestion
Consider adding error handling for profile creation.
While the changes maintain functionality, there are a few concerns:
- If profile creation fails, the user will be created but left without a profile
- No explicit transaction boundary for the two operations
Consider wrapping both operations in a transaction and adding error handling:
- newUser = saveUser(newUser); - learnerProfileService.createProfile(newUser); + try { + newUser = saveUser(newUser); + learnerProfileService.createProfile(newUser); + } catch (Exception e) { + log.error("Failed to create user profile for user {}: {}", newUser.getLogin(), e.getMessage()); + throw new RuntimeException("Failed to complete user creation", e); + }Committable suggestion skipped: line range outside the PR's diff.
src/main/java/de/tum/cit/aet/artemis/atlas/service/learningpath/LearningPathService.java (1)
120-123:
⚠️ Potential issueConsider adding transaction management for data consistency.
The method performs multiple database operations (profile creation and learning path generation) without explicit transaction management. This could lead to inconsistent states if any operation fails.
Consider wrapping the operations in a transaction:
+ @Transactional public void enableLearningPathsForCourse(@NotNull Course course) { course.setLearningPathsEnabled(true); var students = userRepository.getStudentsWithLearnerProfile(course); courseLearnerProfileService.createCourseLearnerProfiles(course, students); generateLearningPaths(course, students); courseRepository.save(course); log.debug("Enabled learning paths for course (id={})", course.getId()); }
Committable suggestion skipped: line range outside the PR's diff.
src/test/java/de/tum/cit/aet/artemis/core/user/util/UserUtilService.java (1)
549-556: 🛠️ Refactor suggestion
Add documentation, validation, and error handling.
The method needs several improvements to meet the test utility standards:
- Add JavaDoc documentation to describe the method's purpose, parameters, and behavior
- Add validation for the
userPrefix
parameter- Add error handling for database operations
- Add assertions to verify the operation's success
Here's the suggested implementation:
+ /** + * Creates and saves learner profiles for all users whose login starts with the given prefix. + * This method is used to set up test data for integration tests. + * + * @param userPrefix The prefix to filter users by (must not be null) + * @throws IllegalArgumentException if userPrefix is null + */ public void createLearnerProfilesForUsers(String userPrefix) { + assertThat(userPrefix).as("userPrefix must not be null").isNotNull(); + Set<LearnerProfile> learnerProfiles = userTestRepository.findAllByUserPrefix(userPrefix).stream().map(user -> { LearnerProfile learnerProfile = new LearnerProfile(); learnerProfile.setUser(user); return learnerProfile; }).collect(Collectors.toSet()); - learnerProfileRepository.saveAll(learnerProfiles); + Set<LearnerProfile> savedProfiles = learnerProfileRepository.saveAll(learnerProfiles); + assertThat(savedProfiles).as("All learner profiles should be saved successfully") + .hasSameSizeAs(learnerProfiles); }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements./** * Creates and saves learner profiles for all users whose login starts with the given prefix. * This method is used to set up test data for integration tests. * * @param userPrefix The prefix to filter users by (must not be null) * @throws IllegalArgumentException if userPrefix is null */ public void createLearnerProfilesForUsers(String userPrefix) { assertThat(userPrefix).as("userPrefix must not be null").isNotNull(); Set<LearnerProfile> learnerProfiles = userTestRepository.findAllByUserPrefix(userPrefix).stream().map(user -> { LearnerProfile learnerProfile = new LearnerProfile(); learnerProfile.setUser(user); return learnerProfile; }).collect(Collectors.toSet()); Set<LearnerProfile> savedProfiles = learnerProfileRepository.saveAll(learnerProfiles); assertThat(savedProfiles).as("All learner profiles should be saved successfully") .hasSameSizeAs(learnerProfiles); }
src/main/java/de/tum/cit/aet/artemis/core/service/user/UserService.java (1)
471-471: 🛠️ Refactor suggestion
Consider adding error handling for profile deletion.
The profile deletion operation could fail and prevent user deletion. Consider wrapping it in a try-catch block to ensure the user deletion process continues even if profile deletion fails.
- learnerProfileService.deleteProfile(user); + try { + learnerProfileService.deleteProfile(user); + } catch (Exception e) { + log.error("Failed to delete learner profile for user {}: {}", user.getLogin(), e.getMessage()); + // Continue with user deletion despite profile deletion failure + }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.try { learnerProfileService.deleteProfile(user); } catch (Exception e) { log.error("Failed to delete learner profile for user {}: {}", user.getLogin(), e.getMessage()); // Continue with user deletion despite profile deletion failure }
src/main/java/de/tum/cit/aet/artemis/atlas/service/learningpath/LearningPathRecommendationService.java (3)
586-591:
⚠️ Potential issueCorrect logical error in exercise selection to prevent negative
remainingExercisePoints
The predicate
remainingExercisePoints.getAndAdd(-exercise.getMaxPoints()) >= 0
may include exercises even whenremainingExercisePoints
are insufficient, causing it to become negative. This could lead to incorrect scheduling of exercises.Apply this diff to fix the predicate logic:
-Predicate<Exercise> exercisePredicate = exercise -> remainingExercisePoints.getAndAdd(-exercise.getMaxPoints()) >= 0; +Predicate<Exercise> exercisePredicate = exercise -> { + double remaining = remainingExercisePoints.get(); + if (remaining >= exercise.getMaxPoints()) { + remainingExercisePoints.addAndGet(-exercise.getMaxPoints()); + return true; + } else { + return false; + } +};Committable suggestion skipped: line range outside the PR's diff.
486-488:
⚠️ Potential issueHandle potential null values for
LearnerProfile
to avoidNullPointerException
The code assumes that
user.getLearnerProfile()
returns a non-null value. IfgetLearnerProfile()
can returnnull
, this will cause aNullPointerException
when callinglearnerProfile.getCourseLearnerProfiles()
. Please add a null check forlearnerProfile
.Apply this diff to handle the potential
null
value:var learnerProfile = user.getLearnerProfile(); +if (learnerProfile == null) { + // Initialize learnerProfile or handle the null case appropriately + learnerProfile = new LearnerProfile(user); +} var courseLearnerProfile = learnerProfile.getCourseLearnerProfiles().stream().findFirst().orElse(new CourseLearnerProfile());Committable suggestion skipped: line range outside the PR's diff.
624-635:
⚠️ Potential issueHandle potential unexpected values in
aimForGradeOrBonus
The methods
getExerciseOrderComparator
andgetExerciseSelectionPredicate
rely onaimForGradeOrBonus
matching specific enum values. Ensure thataimForGradeOrBonus
is validated or handle unexpected values to prevent potential bugs.Apply this diff to include validation:
private static Predicate<Exercise> getExerciseSelectionPredicate(int aimForGradeOrBonus, AtomicDouble remainingExercisePoints) { + if (aimForGradeOrBonus != LOW.getValue() && aimForGradeOrBonus != MEDIUM_HIGH.getValue() && aimForGradeOrBonus != HIGH.getValue()) { + throw new IllegalArgumentException("Invalid aimForGradeOrBonus value: " + aimForGradeOrBonus); + } Predicate<Exercise> exercisePredicate = exercise -> { // existing logic }; // rest of the method }Committable suggestion skipped: line range outside the PR's diff.
src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java (2)
1072-1074:
⚠️ Potential issueAdd missing check for
course.getLearningPathsEnabled()
inremoveUserFromGroup
Currently, the method deletes the course learner profile without verifying if learning paths are enabled for the course. This could lead to unintended behavior or errors when learning paths are not used.
Apply this diff to include the check:
if (group.equals(course.getStudentGroupName())) { + if (course.getLearningPathsEnabled()) { courseLearnerProfileService.deleteCourseLearnerProfile(course, user); + } }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if (group.equals(course.getStudentGroupName())) { if (course.getLearningPathsEnabled()) { courseLearnerProfileService.deleteCourseLearnerProfile(course, user); } }
674-674:
⚠️ Potential issueVerify if learning paths are enabled before deleting learner profiles in
unenrollUserForCourseOrThrow
When unenrolling a user, the method deletes the course learner profile without checking if learning paths are enabled. This may cause issues if learning paths are not enabled for the course.
Consider adding the following check:
+if (course.getLearningPathsEnabled()) { courseLearnerProfileService.deleteCourseLearnerProfile(course, user); +}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if (course.getLearningPathsEnabled()) { courseLearnerProfileService.deleteCourseLearnerProfile(course, user); }
src/main/java/de/tum/cit/aet/artemis/atlas/repository/LearningPathRepository.java (2)
77-77:
⚠️ Potential issueFix WHERE clause to correctly filter by
LearningPath
IDThe query filters by
lp.id = :learningPathId
, which uses theLearnerProfile
ID. However, the parameterlearningPathId
represents the ID of theLearningPath
. This mismatch will result in incorrect data retrieval. Update the WHERE clause to filter byl.id = :learningPathId
.Apply this diff to correct the WHERE clause:
- WHERE lp.id = :learningPathId + WHERE l.id = :learningPathId📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.WHERE l.id = :learningPathId
76-76:
⚠️ Potential issueInvalid use of
LEFT JOIN FETCH
withON
clause in JPQLJPQL does not support
ON
clauses withJOIN FETCH
. The lineLEFT JOIN FETCH lp.courseLearnerProfiles clp ON clp.course.id = l.course.id
is invalid and will cause a runtime exception. Consider removing theFETCH
keyword or restructuring the query.Apply this diff to correct the query:
- LEFT JOIN FETCH lp.courseLearnerProfiles clp ON clp.course.id = l.course.id + LEFT JOIN lp.courseLearnerProfiles clp + WHERE clp.course.id = l.course.idAdjust the query logic to ensure the necessary associations are fetched appropriately.
Committable suggestion skipped: line range outside the PR's diff.
…/adaptive-learning/learner-profile-graded-exercises # Conflicts: # src/main/java/de/tum/cit/aet/artemis/atlas/repository/CourseLearnerProfileRepository.java
…/adaptive-learning/learner-profile-graded-exercises
…nto feature/adaptive-learning/learner-profile-graded-exercises
…/adaptive-learning/learner-profile-graded-exercises
…/adaptive-learning/learner-profile-graded-exercises
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code lgtm for this Pr, on TS5 the same issue which isnt caused by this PR as mentioned above, the exercises loads if you press the compentency drop down though, also the exercises do not start with a server error.
…/adaptive-learning/learner-profile-graded-exercises # Conflicts: # src/main/java/de/tum/cit/aet/artemis/atlas/service/learningpath/LearningPathService.java # src/test/java/de/tum/cit/aet/artemis/atlas/service/LearningPathServiceTest.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on TS5, everything works fine. The issue mentioned above is not related to this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reapprove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on TS4. Everything worked fine for me. (The issue mentioned above doesn't belong to this PR)
Checklist
General
Server
Motivation and Context
Stacked on #9673
#8758
Description
Adapted scheduling of exercises in the learning path to respect the preferences regarding the inclusion of graded exercises.
This PR does not add any UI!
Steps for Testing
Prerequisites:
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Performance Review
Code Review
Manual Tests
Exam Mode Test
Performance Tests
Test Coverage
Summary by CodeRabbit
New Features
CourseLearnerProfile
andLearnerProfile
entities for managing learner profiles in relation to courses.Bug Fixes
Documentation
Tests