Skip to content
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

Tutorial groups: Fix a usability issue when creating new tutorial for campus and language suggestions #7389

Merged
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,14 @@
@Query("""
SELECT DISTINCT tutorialGroup.campus
FROM TutorialGroup tutorialGroup
WHERE tutorialGroup.course.id = :#{#courseId} AND tutorialGroup.campus IS NOT NULL""")
Set<String> findAllUniqueCampusValuesInCourse(@Param("courseId") Long courseId);
WHERE tutorialGroup.course.instructorGroupName IN (:#{#userGroups}) AND tutorialGroup.campus IS NOT NULL""")
Set<String> findAllUniqueCampusValuesInRegisteredCourse(@Param("userGroups") Set<String> userGroups);

Check warning on line 45 in src/main/java/de/tum/in/www1/artemis/repository/tutorialgroups/TutorialGroupRepository.java

View check run for this annotation

Teamscale / teamscale-findings

src/main/java/de/tum/in/www1/artemis/repository/tutorialgroups/TutorialGroupRepository.java#L45

In this file 24 interface comments are missing. Consider adding explanatory comments or restricting the visibility. View in Teamscale: https://teamscale.io/activity.html#details/GitHub-ls1intum-Artemis?t=Tutorial%2Ftutorial-usability-issues-in-the-creation-of-tutorial%3A1698685097000

@Query("""
SELECT DISTINCT tutorialGroup.language
FROM TutorialGroup tutorialGroup
WHERE tutorialGroup.course.id = :#{#courseId} AND tutorialGroup.language IS NOT NULL""")
Set<String> findAllUniqueLanguageValuesInCourse(@Param("courseId") Long courseId);
WHERE tutorialGroup.course.instructorGroupName IN (:#{#userGroups}) AND tutorialGroup.language IS NOT NULL""")
Set<String> findAllUniqueLanguageValuesInRegisteredCourse(@Param("userGroups") Set<String> userGroups);

@Query("""
SELECT tutorialGroup
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,37 +116,39 @@ public ResponseEntity<String> getTitle(@PathVariable Long tutorialGroupId) {
}

/**
* GET /courses/:courseId/tutorial-groups/campus-values : gets the campus values used for the tutorial groups of the course with the given id
* GET /courses/:courseId/tutorial-groups/campus-values : gets the campus values used for the tutorial groups of all tutorials where user is instructor
* Note: Used for autocomplete in the client tutorial form
*
* @param courseId the id of the course to which the tutorial groups belong to
* @return ResponseEntity with status 200 (OK) and with body containing the unique campus values of the tutorial groups of the course
* @return ResponseEntity with status 200 (OK) and with body containing the unique campus values of all tutorials where user is instructor
*/
@GetMapping("/courses/{courseId}/tutorial-groups/campus-values")
@EnforceAtLeastInstructor
@FeatureToggle(Feature.TutorialGroups)
public ResponseEntity<Set<String>> getUniqueCampusValues(@PathVariable Long courseId) {
log.debug("REST request to get unique campus values used for tutorial groups in course : {}", courseId);
var course = courseRepository.findByIdElseThrow(courseId);
authorizationCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.INSTRUCTOR, course, null);
return ResponseEntity.ok(tutorialGroupRepository.findAllUniqueCampusValuesInCourse(courseId));
var user = userRepository.getUserWithGroupsAndAuthorities();
authorizationCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.INSTRUCTOR, course, user);
return ResponseEntity.ok(tutorialGroupRepository.findAllUniqueCampusValuesInRegisteredCourse(user.getGroups()));
}

/**
* GET /courses/:courseId/tutorial-groups/language-values : gets the language values used for the tutorial groups of the course with the given id
* GET /courses/:courseId/tutorial-groups/language-values : gets the language values used for the tutorial groups of all tutorials where user is instructor
* Note: Used for autocomplete in the client tutorial form
*
* @param courseId the id of the course to which the tutorial groups belong to
* @return ResponseEntity with status 200 (OK) and with body containing the unique language values of the tutorial groups of the course
* @return ResponseEntity with status 200 (OK) and with body containing the unique language values of all tutorials where user is instructor
*/
@GetMapping("/courses/{courseId}/tutorial-groups/language-values")
@EnforceAtLeastInstructor
@FeatureToggle(Feature.TutorialGroups)
public ResponseEntity<Set<String>> getUniqueLanguageValues(@PathVariable Long courseId) {
log.debug("REST request to get unique language values used for tutorial groups in course : {}", courseId);
var course = courseRepository.findByIdElseThrow(courseId);
authorizationCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.INSTRUCTOR, course, null);
return ResponseEntity.ok(tutorialGroupRepository.findAllUniqueLanguageValuesInCourse(courseId));
var user = userRepository.getUserWithGroupsAndAuthorities();
authorizationCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.INSTRUCTOR, course, user);
return ResponseEntity.ok(tutorialGroupRepository.findAllUniqueLanguageValuesInRegisteredCourse(user.getGroups()));
}

/**
Expand All @@ -161,7 +163,7 @@ public ResponseEntity<Set<String>> getUniqueLanguageValues(@PathVariable Long co
public ResponseEntity<List<TutorialGroup>> getAllForCourse(@PathVariable Long courseId) {
log.debug("REST request to get all tutorial groups of course with id: {}", courseId);
var course = courseRepository.findByIdElseThrow(courseId);
var user = userRepository.getUserWithGroupsAndAuthorities();
var user = userRepository.getUser();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The groups are needed in the next call (checkHasAtLeastRoleInCourseElseThrow), which currently leads to an additional database request.

authorizationCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.STUDENT, course, user);
// ToDo: Optimization Idea: Do not send all registered student information but just the number in a DTO
var tutorialGroups = tutorialGroupService.findAllForCourse(course, user);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ export class CreateTutorialGroupComponent implements OnInit, OnDestroy {

createTutorialGroup(formData: TutorialGroupFormData) {
const { title, teachingAssistant, additionalInformation, capacity, isOnline, language, campus, schedule } = formData;

this.tutorialGroupToCreate.title = title;
this.tutorialGroupToCreate.teachingAssistant = teachingAssistant;
this.tutorialGroupToCreate.additionalInformation = additionalInformation;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ export class TutorialGroupFormComponent implements OnInit, OnChanges, OnDestroy
teachingAssistant: [undefined, [Validators.required]],
capacity: [undefined, [Validators.min(1)]],
isOnline: [false, [Validators.required]],
language: ['German', [Validators.required, Validators.maxLength(255)]],
language: [undefined, [Validators.required, Validators.maxLength(255)]],
campus: [undefined, Validators.maxLength(255)],
});

Expand Down Expand Up @@ -367,6 +367,13 @@ export class TutorialGroupFormComponent implements OnInit, OnChanges, OnDestroy
),
).subscribe((languages: string[]) => {
this.languages = languages;
// default values for English & German
if (!languages.includes('English')) {
this.languages.push('English');
}
if (!languages.includes('German')) {
this.languages.push('German');
}
});
}
}
Loading