Skip to content

Commit 67daa30

Browse files
kruschekrusche
authored
Atlas: Fix issues in course learner profile settings (#10840)
1 parent 6b96b0a commit 67daa30

File tree

13 files changed

+179
-156
lines changed

13 files changed

+179
-156
lines changed

‎jest.config.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,10 @@ module.exports = {
9393
coverageThreshold: {
9494
global: {
9595
// TODO: in the future, the following values should increase to at least 90%
96-
statements: 89.18,
97-
branches: 75.32,
98-
functions: 83.07,
99-
lines: 89.25,
96+
statements: 89.15,
97+
branches: 75.26,
98+
functions: 83.01,
99+
lines: 89.22,
100100
},
101101
},
102102
coverageReporters: ['clover', 'json', 'lcov', 'text-summary'],

‎src/main/java/de/tum/cit/aet/artemis/atlas/domain/profile/CourseLearnerProfile.java

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,16 @@ public class CourseLearnerProfile extends DomainObject {
2626

2727
public static final String ENTITY_NAME = "courseLearnerProfile";
2828

29+
/**
30+
* Minimum value allowed for profile fields representing values on a Likert scale.
31+
*/
32+
public static final int MIN_PROFILE_VALUE = 1;
33+
34+
/**
35+
* Maximum value allowed for profile fields representing values on a Likert scale.
36+
*/
37+
public static final int MAX_PROFILE_VALUE = 5;
38+
2939
@ManyToOne
3040
@JoinColumn(name = "learner_profile_id")
3141
private LearnerProfile learnerProfile;
@@ -35,18 +45,18 @@ public class CourseLearnerProfile extends DomainObject {
3545
private Course course;
3646

3747
@Column(name = "aim_for_grade_or_bonus")
38-
@Min(0)
39-
@Max(5)
48+
@Min(0) // TODO should actually be MIN_PROFILE_VALUE = 1, however, then almost all tests in LearningPathIntegrationTest fail
49+
@Max(MAX_PROFILE_VALUE)
4050
private int aimForGradeOrBonus;
4151

4252
@Column(name = "time_investment")
43-
@Min(0)
44-
@Max(5)
53+
@Min(0) // TODO should actually be MIN_PROFILE_VALUE = 1, however, then almost all tests in LearningPathIntegrationTest fail
54+
@Max(MAX_PROFILE_VALUE)
4555
private int timeInvestment;
4656

4757
@Column(name = "repetition_intensity")
48-
@Min(0)
49-
@Max(5)
58+
@Min(0) // TODO should actually be MIN_PROFILE_VALUE = 1, however, then almost all tests in LearningPathIntegrationTest fail
59+
@Max(MAX_PROFILE_VALUE)
5060
private int repetitionIntensity;
5161

5262
public void setLearnerProfile(LearnerProfile learnerProfile) {

‎src/main/java/de/tum/cit/aet/artemis/atlas/dto/CourseLearnerProfileDTO.java

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
package de.tum.cit.aet.artemis.atlas.dto;
22

3+
import static de.tum.cit.aet.artemis.atlas.domain.profile.CourseLearnerProfile.MAX_PROFILE_VALUE;
4+
import static de.tum.cit.aet.artemis.atlas.domain.profile.CourseLearnerProfile.MIN_PROFILE_VALUE;
5+
6+
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
37
import com.fasterxml.jackson.annotation.JsonInclude;
48

59
import de.tum.cit.aet.artemis.atlas.domain.profile.CourseLearnerProfile;
610

711
@JsonInclude(JsonInclude.Include.NON_EMPTY)
8-
public record CourseLearnerProfileDTO(long id, long courseId, int aimForGradeOrBonus, int timeInvestment, int repetitionIntensity) {
12+
@JsonIgnoreProperties(ignoreUnknown = true)
13+
public record CourseLearnerProfileDTO(long id, long courseId, String courseTitle, int aimForGradeOrBonus, int timeInvestment, int repetitionIntensity) {
914

1015
/**
1116
* Creates CourseLearnerProfileDTO from given CourseLearnerProfile.
@@ -14,7 +19,18 @@ public record CourseLearnerProfileDTO(long id, long courseId, int aimForGradeOrB
1419
* @return CourseLearnerProfile DTO for transfer
1520
*/
1621
public static CourseLearnerProfileDTO of(CourseLearnerProfile courseLearnerProfile) {
17-
return new CourseLearnerProfileDTO(courseLearnerProfile.getId(), courseLearnerProfile.getCourse().getId(), courseLearnerProfile.getAimForGradeOrBonus(),
18-
courseLearnerProfile.getTimeInvestment(), courseLearnerProfile.getRepetitionIntensity());
22+
var course = courseLearnerProfile.getCourse();
23+
return new CourseLearnerProfileDTO(courseLearnerProfile.getId(), course.getId(), course.getTitle(), clamp(courseLearnerProfile.getAimForGradeOrBonus()),
24+
clamp(courseLearnerProfile.getTimeInvestment()), clamp(courseLearnerProfile.getRepetitionIntensity()));
25+
}
26+
27+
/**
28+
* Clamps the given value to be within the range of {@link #MIN_PROFILE_VALUE} and {@link #MAX_PROFILE_VALUE}.
29+
*
30+
* @param value The value to clamp
31+
* @return The clamped value
32+
*/
33+
private static int clamp(int value) {
34+
return Math.max(MIN_PROFILE_VALUE, Math.min(MAX_PROFILE_VALUE, value));
1935
}
2036
}

‎src/main/java/de/tum/cit/aet/artemis/atlas/repository/CourseLearnerProfileRepository.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package de.tum.cit.aet.artemis.atlas.repository;
22

3+
import java.util.Optional;
34
import java.util.Set;
45

56
import org.springframework.context.annotation.Conditional;
@@ -34,7 +35,17 @@ public interface CourseLearnerProfileRepository extends ArtemisJpaRepository<Cou
3435
@Query("""
3536
SELECT clp
3637
FROM CourseLearnerProfile clp
38+
LEFT JOIN FETCH clp.course
3739
WHERE clp.learnerProfile.user.login = :login
3840
""")
3941
Set<CourseLearnerProfile> findAllByLogin(@Param("login") String login);
42+
43+
@Query("""
44+
SELECT clp
45+
FROM CourseLearnerProfile clp
46+
LEFT JOIN FETCH clp.course
47+
WHERE clp.learnerProfile.user.login = :login
48+
AND clp.id = :courseLearnerProfileId
49+
""")
50+
Optional<CourseLearnerProfile> findByLoginAndId(@Param("login") String login, @Param("courseLearnerProfileId") long courseLearnerProfileId);
4051
}

‎src/main/java/de/tum/cit/aet/artemis/atlas/web/LearnerProfileResource.java

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package de.tum.cit.aet.artemis.atlas.web;
22

3-
import java.util.Map;
3+
import static de.tum.cit.aet.artemis.atlas.domain.profile.CourseLearnerProfile.MAX_PROFILE_VALUE;
4+
import static de.tum.cit.aet.artemis.atlas.domain.profile.CourseLearnerProfile.MIN_PROFILE_VALUE;
5+
46
import java.util.Optional;
57
import java.util.Set;
68
import java.util.stream.Collectors;
@@ -30,16 +32,6 @@
3032
@RequestMapping("api/atlas/")
3133
public class LearnerProfileResource {
3234

33-
/**
34-
* Minimum value allowed for profile fields representing values on a Likert scale.
35-
*/
36-
private static final int MIN_PROFILE_VALUE = 1;
37-
38-
/**
39-
* Maximum value allowed for profile fields representing values on a Likert scale.
40-
*/
41-
private static final int MAX_PROFILE_VALUE = 5;
42-
4335
private static final Logger log = LoggerFactory.getLogger(LearnerProfileResource.class);
4436

4537
private final UserRepository userRepository;
@@ -52,19 +44,19 @@ public LearnerProfileResource(UserRepository userRepository, CourseLearnerProfil
5244
}
5345

5446
/**
55-
* GET /learner-profiles/course-learner-profiles : get a Map of a {@link de.tum.cit.aet.artemis.core.domain.Course} id
47+
* GET course-learner-profiles : get a Map of a {@link de.tum.cit.aet.artemis.core.domain.Course} id
5648
* to the corresponding {@link CourseLearnerProfile} of the logged-in user.
5749
*
5850
* @return The ResponseEntity with status 200 (OK) and with the body containing a map of DTOs, which contains per course profile data.
5951
*/
6052
@GetMapping("course-learner-profiles")
6153
@EnforceAtLeastStudent
62-
public ResponseEntity<Map<Long, CourseLearnerProfileDTO>> getCourseLearnerProfiles() {
54+
public ResponseEntity<Set<CourseLearnerProfileDTO>> getCourseLearnerProfiles() {
6355
User user = userRepository.getUser();
6456
log.debug("REST request to get all CourseLearnerProfiles of user {}", user.getLogin());
65-
Map<Long, CourseLearnerProfileDTO> result = courseLearnerProfileRepository.findAllByLogin(user.getLogin()).stream()
66-
.collect(Collectors.toMap(profile -> profile.getCourse().getId(), CourseLearnerProfileDTO::of));
67-
return ResponseEntity.ok(result);
57+
Set<CourseLearnerProfileDTO> courseLearnerProfiles = courseLearnerProfileRepository.findAllByLogin(user.getLogin()).stream().map(CourseLearnerProfileDTO::of)
58+
.collect(Collectors.toSet());
59+
return ResponseEntity.ok(courseLearnerProfiles);
6860
}
6961

7062
/**
@@ -75,12 +67,13 @@ public ResponseEntity<Map<Long, CourseLearnerProfileDTO>> getCourseLearnerProfil
7567
*/
7668
private void validateProfileField(int value, String fieldName) {
7769
if (value < MIN_PROFILE_VALUE || value > MAX_PROFILE_VALUE) {
78-
throw new BadRequestAlertException(fieldName + " field is outside valid bounds", CourseLearnerProfile.ENTITY_NAME, fieldName.toLowerCase() + "OutOfBounds", true);
70+
String message = String.format("%s (%d) is outside valid bounds [%d, %d]", fieldName, value, MIN_PROFILE_VALUE, MAX_PROFILE_VALUE);
71+
throw new BadRequestAlertException(message, CourseLearnerProfile.ENTITY_NAME, fieldName.toLowerCase() + "OutOfBounds", true);
7972
}
8073
}
8174

8275
/**
83-
* PUT /learner-profiles/course-learner-profiles/{courseLearnerProfileId} : update fields in a {@link CourseLearnerProfile}.
76+
* PUT course-learner-profiles/{courseLearnerProfileId} : update fields in a {@link CourseLearnerProfile}.
8477
*
8578
* @param courseLearnerProfileId ID of the CourseLearnerProfile
8679
* @param courseLearnerProfileDTO {@link CourseLearnerProfileDTO} object from the request body.
@@ -98,9 +91,7 @@ public ResponseEntity<CourseLearnerProfileDTO> updateCourseLearnerProfile(@PathV
9891
true);
9992
}
10093

101-
Set<CourseLearnerProfile> clps = courseLearnerProfileRepository.findAllByLogin(user.getLogin());
102-
Optional<CourseLearnerProfile> optionalCourseLearnerProfile = clps.stream()
103-
.filter(clp -> clp.getId() == courseLearnerProfileId && clp.getCourse().getId() == courseLearnerProfileDTO.courseId()).findFirst();
94+
Optional<CourseLearnerProfile> optionalCourseLearnerProfile = courseLearnerProfileRepository.findByLoginAndId(user.getLogin(), courseLearnerProfileId);
10495

10596
if (optionalCourseLearnerProfile.isEmpty()) {
10697
throw new BadRequestAlertException("CourseLearnerProfile not found.", CourseLearnerProfile.ENTITY_NAME, "courseLearnerProfileNotFound", true);
@@ -115,7 +106,7 @@ public ResponseEntity<CourseLearnerProfileDTO> updateCourseLearnerProfile(@PathV
115106
updateProfile.setTimeInvestment(courseLearnerProfileDTO.timeInvestment());
116107
updateProfile.setRepetitionIntensity(courseLearnerProfileDTO.repetitionIntensity());
117108

118-
CourseLearnerProfile result = courseLearnerProfileRepository.save(updateProfile);
119-
return ResponseEntity.ok(CourseLearnerProfileDTO.of(result));
109+
courseLearnerProfileRepository.save(updateProfile);
110+
return ResponseEntity.ok(CourseLearnerProfileDTO.of(updateProfile));
120111
}
121112
}

‎src/main/webapp/app/core/user/settings/learner-profile/course-learner-profile.component.html

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ <h2 jhiTranslate="artemisApp.learnerProfile.courseLearnerProfile.title"></h2>
2222
</div>
2323
<select class="form-select col-auto w-auto" (change)="courseChanged($event)">
2424
<option value="-1" jhiTranslate="artemisApp.learnerProfile.courseLearnerProfile.selectCourse" selected></option>
25-
@for (course of courses; track course.id) {
26-
<option value="{{ course.id }}">{{ course.title }}</option>
25+
@for (courseLearnerProfile of courseLearnerProfiles; track courseLearnerProfile.courseId) {
26+
<option value="{{ courseLearnerProfile.courseId }}">{{ courseLearnerProfile.courseTitle }}</option>
2727
}
2828
</select>
2929
</div>
@@ -32,8 +32,8 @@ <h2 jhiTranslate="artemisApp.learnerProfile.courseLearnerProfile.title"></h2>
3232
<div class="col">
3333
<jhi-double-slider
3434
[title]="translateService.instant('artemisApp.learnerProfile.courseLearnerProfile.aimForGradeOrBonus')"
35-
[min]="1"
36-
[max]="5"
35+
[min]="MIN_PROFILE_VALUE"
36+
[max]="MAX_PROFILE_VALUE"
3737
[initialValue]="initialAimForGradeOrBonus"
3838
[(currentValue)]="aimForGradeOrBonus"
3939
(currentValueChange)="this.editing = true"
@@ -42,8 +42,8 @@ <h2 jhiTranslate="artemisApp.learnerProfile.courseLearnerProfile.title"></h2>
4242
<div class="col">
4343
<jhi-double-slider
4444
[title]="translateService.instant('artemisApp.learnerProfile.courseLearnerProfile.timeInvestment')"
45-
[min]="1"
46-
[max]="5"
45+
[min]="MIN_PROFILE_VALUE"
46+
[max]="MAX_PROFILE_VALUE"
4747
[initialValue]="initialTimeInvestment"
4848
[(currentValue)]="timeInvestment"
4949
(currentValueChange)="this.editing = true"
@@ -52,8 +52,8 @@ <h2 jhiTranslate="artemisApp.learnerProfile.courseLearnerProfile.title"></h2>
5252
<div class="col align-items-end">
5353
<jhi-double-slider
5454
[title]="translateService.instant('artemisApp.learnerProfile.courseLearnerProfile.repetitionIntensity')"
55-
[min]="1"
56-
[max]="5"
55+
[min]="MIN_PROFILE_VALUE"
56+
[max]="MAX_PROFILE_VALUE"
5757
[initialValue]="initialRepetitionIntensity"
5858
[(currentValue)]="repetitionIntensity"
5959
(currentValueChange)="this.editing = true"

‎src/main/webapp/app/core/user/settings/learner-profile/course-learner-profile.component.spec.ts

Lines changed: 29 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -53,18 +53,20 @@ describe('CourseLearnerProfileComponent', () => {
5353
const clp1: CourseLearnerProfileDTO = {
5454
id: 1,
5555
courseId: 1,
56+
courseTitle: 'Course 1',
5657
aimForGradeOrBonus: 0,
5758
timeInvestment: 0,
5859
repetitionIntensity: 0,
5960
};
6061
const clp2: CourseLearnerProfileDTO = {
6162
id: 2,
6263
courseId: 2,
64+
courseTitle: 'Course 2',
6365
aimForGradeOrBonus: 1,
6466
timeInvestment: 1,
6567
repetitionIntensity: 1,
6668
};
67-
const profiles = { [clp1.courseId]: clp1, [clp2.courseId]: clp2 };
69+
const profiles = [clp1, clp2];
6870

6971
beforeEach(async () => {
7072
await TestBed.configureTestingModule({
@@ -99,9 +101,7 @@ describe('CourseLearnerProfileComponent', () => {
99101
);
100102
});
101103

102-
jest.spyOn(learnerProfileApiService, 'getCourseLearnerProfilesForCurrentUser').mockReturnValue(
103-
new Promise<Record<number, CourseLearnerProfileDTO>>((resolve) => resolve(profiles)),
104-
);
104+
jest.spyOn(learnerProfileApiService, 'getCourseLearnerProfilesForCurrentUser').mockResolvedValue(profiles as CourseLearnerProfileDTO[]);
105105

106106
putUpdatedCourseLearnerProfileSpy = jest.spyOn(learnerProfileApiService, 'putUpdatedCourseLearnerProfile');
107107

@@ -124,7 +124,6 @@ describe('CourseLearnerProfileComponent', () => {
124124

125125
it('should initialize', () => {
126126
expect(component).toBeTruthy();
127-
expect(component.courses).toStrictEqual(courses);
128127
expect(component.courseLearnerProfiles).toEqual(profiles);
129128
});
130129

@@ -133,52 +132,57 @@ describe('CourseLearnerProfileComponent', () => {
133132
selectCourse(course);
134133
const changeEvent = new Event('change', { bubbles: true, cancelable: false });
135134
selector.dispatchEvent(changeEvent);
136-
expect(component.activeCourse).toBe(course);
135+
expect(component.activeCourseId).toBe(course);
137136
});
138137

139-
function setupUpdateTest(course: number): CourseLearnerProfileDTO {
140-
const newProfile = profiles[course];
141-
newProfile['repetitionIntensity'] = 1;
142-
newProfile['aimForGradeOrBonus'] = 2;
143-
newProfile['timeInvestment'] = 3;
144-
component.activeCourse = course;
138+
function setupUpdateTest(courseIndex: number, courseId: number, mockUpdate: boolean): CourseLearnerProfileDTO {
139+
const newProfile = { ...profiles[courseIndex] };
140+
newProfile.repetitionIntensity = 1;
141+
newProfile.aimForGradeOrBonus = 2;
142+
newProfile.timeInvestment = 3;
143+
144+
// Inject into component state
145+
component.courseLearnerProfiles[courseIndex] = newProfile;
146+
component.activeCourseId = courseId;
147+
148+
if (mockUpdate) {
149+
putUpdatedCourseLearnerProfileSpy.mockResolvedValue(newProfile);
150+
}
145151

146152
component.update();
147153

148154
return newProfile;
149155
}
150156

151-
function validateUpdate(course: number, profile: CourseLearnerProfileDTO) {
152-
const req = httpTesting.expectOne(`api/atlas/course-learner-profiles/${course}`, 'Request to put new Profile');
153-
req.flush(profile);
157+
function validateUpdate(index: number, profile: CourseLearnerProfileDTO) {
154158
expect(putUpdatedCourseLearnerProfileSpy).toHaveBeenCalled();
155159
expect(putUpdatedCourseLearnerProfileSpy.mock.calls[0][0]).toEqual(profile);
156-
expect(component.courseLearnerProfiles[course]).toEqual(profile);
160+
expect(component.courseLearnerProfiles[index]).toEqual(profile);
157161
}
158162

159-
function validateError(course: number, profile: CourseLearnerProfileDTO) {
160-
const req = httpTesting.expectOne(`api/atlas/course-learner-profiles/${course}`, 'Request to put new Profile');
163+
function validateError(courseId: number, index: number, profile: CourseLearnerProfileDTO) {
164+
const req = httpTesting.expectOne(`api/atlas/course-learner-profiles/${courseId}`, 'Request to put new Profile');
161165
req.flush(errorBody, {
162166
headers: errorHeaders,
163167
status: 400,
164168
statusText: 'Bad Request',
165169
});
166170
expect(putUpdatedCourseLearnerProfileSpy).toHaveBeenCalled();
167171
expect(putUpdatedCourseLearnerProfileSpy.mock.calls[0][0]).toEqual(profile);
168-
expect(component.courseLearnerProfiles[course]).toEqual(profiles[course]);
172+
expect(component.courseLearnerProfiles[index]).toEqual(profiles[index]);
169173
}
170174

171175
describe('Making put requests', () => {
172176
it('should update profile on successful request', () => {
173-
const course = 1;
174-
const profile = setupUpdateTest(course);
175-
validateUpdate(course, profile);
177+
const courseIndex = 1;
178+
const profile = setupUpdateTest(courseIndex, profiles[courseIndex].courseId, true);
179+
validateUpdate(courseIndex, profile);
176180
});
177181

178182
it('should error on bad request', () => {
179-
const course = 1;
180-
const profile = setupUpdateTest(course);
181-
validateError(course, profile);
183+
const courseIndex = 1;
184+
const profile = setupUpdateTest(courseIndex, profiles[courseIndex].courseId, false);
185+
validateError(profiles[courseIndex].courseId, courseIndex, profile);
182186
});
183187
});
184188
});

0 commit comments

Comments
 (0)