From edce19d60205ccfc1908b2e450c0a716e113dc9c Mon Sep 17 00:00:00 2001 From: Julian Christl Date: Tue, 18 Oct 2022 20:48:38 +0200 Subject: [PATCH 01/16] reimplement course icon file upload --- .../www1/artemis/web/rest/CourseResource.java | 202 +++++++++--------- .../manage/course-management.service.ts | 33 ++- .../manage/course-update.component.html | 6 - .../course/manage/course-update.component.ts | 31 +-- .../app/shared/http/file-uploader.service.ts | 2 +- .../course/course-management.service.spec.ts | 4 +- .../course/course-update.component.spec.ts | 29 --- .../spec/service/course.service.spec.ts | 2 +- 8 files changed, 131 insertions(+), 178 deletions(-) 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 3ebde0da8138..ffa8928ebbfc 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 @@ -30,6 +30,7 @@ import org.springframework.http.ResponseEntity; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.web.bind.annotation.*; +import org.springframework.web.multipart.MultipartFile; import org.springframework.web.server.ResponseStatusException; import org.springframework.web.servlet.support.ServletUriComponentsBuilder; @@ -57,7 +58,7 @@ * REST controller for managing Course. */ @RestController -@RequestMapping("/api") +@RequestMapping("api/") @PreAuthorize("hasRole('ADMIN')") public class CourseResource { @@ -96,10 +97,12 @@ public class CourseResource { private final ExerciseRepository exerciseRepository; + private final FileService fileService; + public CourseResource(UserRepository userRepository, CourseService courseService, CourseRepository courseRepository, ExerciseService exerciseService, AuthorizationCheckService authCheckService, TutorParticipationRepository tutorParticipationRepository, SubmissionService submissionService, AuditEventRepository auditEventRepository, Optional optionalVcsUserManagementService, AssessmentDashboardService assessmentDashboardService, - ExerciseRepository exerciseRepository, Optional optionalCiUserManagementService) { + ExerciseRepository exerciseRepository, Optional optionalCiUserManagementService, FileService fileService) { this.courseService = courseService; this.courseRepository = courseRepository; this.exerciseService = exerciseService; @@ -112,6 +115,7 @@ public CourseResource(UserRepository userRepository, CourseService courseService this.assessmentDashboardService = assessmentDashboardService; this.userRepository = userRepository; this.exerciseRepository = exerciseRepository; + this.fileService = fileService; } /** @@ -121,9 +125,9 @@ public CourseResource(UserRepository userRepository, CourseService courseService * @return the ResponseEntity with status 201 (Created) and with body the new course, or with status 400 (Bad Request) if the course has already an ID * @throws URISyntaxException if the Location URI syntax is incorrect */ - @PostMapping("/courses") + @PostMapping(value = "courses", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) @PreAuthorize("hasRole('ADMIN')") - public ResponseEntity createCourse(@RequestBody Course course) throws URISyntaxException { + public ResponseEntity createCourse(@RequestPart Course course, @RequestPart(required = false) MultipartFile file) throws URISyntaxException { log.debug("REST request to save Course : {}", course); if (course.getId() != null) { throw new BadRequestAlertException("A new course cannot already have an ID", Course.ENTITY_NAME, "idExists"); @@ -148,33 +152,30 @@ public ResponseEntity createCourse(@RequestBody Course course) throws UR } courseService.createOrValidateGroups(course); + + if (file != null) { + String pathString = fileService.handleSaveFile(file, false, false); + course.setCourseIcon(pathString); + } + Course result = courseRepository.save(course); return ResponseEntity.created(new URI("/api/courses/" + result.getId())).body(result); } /** - * PUT /courses : Updates an existing updatedCourse. + * PUT /courses/:id : Updates an existing updatedCourse. * - * @param updatedCourse the course to update + * @param courseUpdate the course to update * @return the ResponseEntity with status 200 (OK) and with body the updated course - * @throws URISyntaxException if the Location URI syntax is incorrect */ - @PutMapping("/courses") + @PutMapping(value = "courses/{id}", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) @PreAuthorize("hasRole('INSTRUCTOR')") - public ResponseEntity updateCourse(@RequestBody Course updatedCourse) throws URISyntaxException { - log.debug("REST request to update Course : {}", updatedCourse); + public ResponseEntity updateCourse(@PathVariable Long id, @RequestPart("course") Course courseUpdate, @RequestPart(required = false) MultipartFile file) { + log.debug("REST request to update Course : {}", courseUpdate); User user = userRepository.getUserWithGroupsAndAuthorities(); - if (updatedCourse.getId() == null) { - if (authCheckService.isAdmin(user)) { - return createCourse(updatedCourse); - } - else { - throw new AccessForbiddenException(); - } - } - var existingCourse = courseRepository.findByIdWithOrganizationsAndLearningGoalsElseThrow(updatedCourse.getId()); - if (!Objects.equals(existingCourse.getShortName(), updatedCourse.getShortName())) { + var existingCourse = courseRepository.findByIdWithOrganizationsAndLearningGoalsElseThrow(id); + if (!Objects.equals(existingCourse.getShortName(), courseUpdate.getShortName())) { throw new BadRequestAlertException("The course short name cannot be changed", Course.ENTITY_NAME, "shortNameCannotChange", true); } @@ -182,21 +183,17 @@ public ResponseEntity updateCourse(@RequestBody Course updatedCourse) th // this is important, otherwise someone could put himself into the instructor group of the updated course authCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.INSTRUCTOR, existingCourse, user); + Set existingGroupNames = Set.of(existingCourse.getStudentGroupName(), existingCourse.getTeachingAssistantGroupName(), existingCourse.getEditorGroupName(), + existingCourse.getInstructorGroupName()); + Set newGroupNames = Set.of(courseUpdate.getStudentGroupName(), courseUpdate.getTeachingAssistantGroupName(), courseUpdate.getEditorGroupName(), + courseUpdate.getInstructorGroupName()); + Set changedGroupNames = new HashSet<>(newGroupNames); + changedGroupNames.removeAll(existingGroupNames); + if (authCheckService.isAdmin(user)) { // if an admin changes a group, we need to check that the changed group exists try { - if (!Objects.equals(existingCourse.getStudentGroupName(), updatedCourse.getStudentGroupName())) { - courseService.checkIfGroupsExists(updatedCourse.getStudentGroupName()); - } - if (!Objects.equals(existingCourse.getTeachingAssistantGroupName(), updatedCourse.getTeachingAssistantGroupName())) { - courseService.checkIfGroupsExists(updatedCourse.getTeachingAssistantGroupName()); - } - if (!Objects.equals(existingCourse.getEditorGroupName(), updatedCourse.getEditorGroupName())) { - courseService.checkIfGroupsExists(updatedCourse.getEditorGroupName()); - } - if (!Objects.equals(existingCourse.getInstructorGroupName(), updatedCourse.getInstructorGroupName())) { - courseService.checkIfGroupsExists(updatedCourse.getInstructorGroupName()); - } + changedGroupNames.forEach(courseService::checkIfGroupsExists); } catch (ArtemisAuthenticationException ex) { // a specified group does not exist, notify the client @@ -206,41 +203,37 @@ public ResponseEntity updateCourse(@RequestBody Course updatedCourse) th else { // this means the user must be an instructor, who has NO Admin rights. // instructors are not allowed to change group names, because this would lead to security problems - - if (!Objects.equals(existingCourse.getStudentGroupName(), updatedCourse.getStudentGroupName())) { - throw new BadRequestAlertException("The student group name cannot be changed", Course.ENTITY_NAME, "studentGroupNameCannotChange", true); - } - if (!Objects.equals(existingCourse.getTeachingAssistantGroupName(), updatedCourse.getTeachingAssistantGroupName())) { - throw new BadRequestAlertException("The teaching assistant group name cannot be changed", Course.ENTITY_NAME, "teachingAssistantGroupNameCannotChange", true); - } - if (!Objects.equals(existingCourse.getEditorGroupName(), updatedCourse.getEditorGroupName())) { - throw new BadRequestAlertException("The editor group name cannot be changed", Course.ENTITY_NAME, "editorGroupNameCannotChange", true); - } - if (!Objects.equals(existingCourse.getInstructorGroupName(), updatedCourse.getInstructorGroupName())) { - throw new BadRequestAlertException("The instructor group name cannot be changed", Course.ENTITY_NAME, "instructorGroupNameCannotChange", true); + if (!changedGroupNames.isEmpty()) { + throw new BadRequestAlertException("You are not allowed to change the group names of a course", Course.ENTITY_NAME, "groupNamesCannotChange", true); } } // Make sure to preserve associations in updated entity - updatedCourse.setPrerequisites(existingCourse.getPrerequisites()); - - updatedCourse.validateRegistrationConfirmationMessage(); - updatedCourse.validateComplaintsAndRequestMoreFeedbackConfig(); - updatedCourse.validateOnlineCourseAndRegistrationEnabled(); - updatedCourse.validateOnlineCourseConfiguration(); - updatedCourse.validateShortName(); - updatedCourse.validateAccuracyOfScores(); - if (!updatedCourse.isValidStartAndEndDate()) { + courseUpdate.setPrerequisites(existingCourse.getPrerequisites()); + + courseUpdate.validateRegistrationConfirmationMessage(); + courseUpdate.validateComplaintsAndRequestMoreFeedbackConfig(); + courseUpdate.validateOnlineCourseAndRegistrationEnabled(); + courseUpdate.validateOnlineCourseConfiguration(); + courseUpdate.validateShortName(); + courseUpdate.validateAccuracyOfScores(); + if (!courseUpdate.isValidStartAndEndDate()) { throw new BadRequestAlertException("For Courses, the start date has to be before the end date", Course.ENTITY_NAME, "invalidCourseStartDate", true); } + if (file != null) { + String pathString = fileService.handleSaveFile(file, false, false); + courseUpdate.setCourseIcon(pathString); + } + + Course result = courseRepository.save(courseUpdate); + // Based on the old instructors, editors and TAs, we can update all exercises in the course in the VCS (if necessary) // We need the old instructors, editors and TAs, so that the VCS user management service can determine which // users no longer have TA, editor or instructor rights in the related exercise repositories. final var oldInstructorGroup = existingCourse.getInstructorGroupName(); final var oldEditorGroup = existingCourse.getEditorGroupName(); final var oldTeachingAssistantGroup = existingCourse.getTeachingAssistantGroupName(); - Course result = courseRepository.save(updatedCourse); optionalVcsUserManagementService .ifPresent(userManagementService -> userManagementService.updateCoursePermissions(result, oldInstructorGroup, oldEditorGroup, oldTeachingAssistantGroup)); optionalCiUserManagementService @@ -256,7 +249,7 @@ public ResponseEntity updateCourse(@RequestBody Course updatedCourse) th * @param courseId to find the course * @return response entity for user who has been registered to the course */ - @PostMapping("/courses/{courseId}/register") + @PostMapping("courses/{courseId}/register") @PreAuthorize("hasRole('USER')") public ResponseEntity registerForCourse(@PathVariable Long courseId) { Course course = courseRepository.findWithEagerOrganizationsElseThrow(courseId); @@ -278,7 +271,7 @@ public ResponseEntity registerForCourse(@PathVariable Long courseId) { } if (!Boolean.TRUE.equals(course.isRegistrationEnabled())) { return ResponseEntity.badRequest().headers(HeaderUtil.createFailureAlert(applicationName, false, Course.ENTITY_NAME, "registrationDisabled", - "The course does not allow registration. Cannot register user")).body(null); + "The course does not allow registration. Cannot register " + "user")).body(null); } if (course.getOrganizations() != null && !course.getOrganizations().isEmpty() && !courseRepository.checkIfUserIsMemberOfCourseOrganizations(user, course)) { return ResponseEntity.badRequest().headers(HeaderUtil.createFailureAlert(applicationName, false, Course.ENTITY_NAME, "registrationNotAllowed", @@ -294,7 +287,7 @@ public ResponseEntity registerForCourse(@PathVariable Long courseId) { * @param onlyActive if true, only active courses will be considered in the result * @return the list of courses (the user has access to) */ - @GetMapping("/courses") + @GetMapping("courses") @PreAuthorize("hasRole('TA')") public List getAllCourses(@RequestParam(defaultValue = "false") boolean onlyActive) { log.debug("REST request to get all Courses the user has access to"); @@ -314,7 +307,7 @@ public List getAllCourses(@RequestParam(defaultValue = "false") boolean * * @return the list of courses */ - @GetMapping("/courses/courses-with-quiz") + @GetMapping("courses/courses-with-quiz") @PreAuthorize("hasRole('EDITOR')") public List getAllCoursesWithQuizExercises() { User user = userRepository.getUserWithGroupsAndAuthorities(); @@ -333,7 +326,7 @@ public List getAllCoursesWithQuizExercises() { * @param onlyActive if true, only active courses will be considered in the result * @return the list of courses (the user has access to) */ - @GetMapping("/courses/with-user-stats") + @GetMapping("courses/with-user-stats") @PreAuthorize("hasRole('TA')") public List getAllCoursesWithUserStats(@RequestParam(defaultValue = "false") boolean onlyActive) { log.debug("get courses with user stats, only active: {}", onlyActive); @@ -353,7 +346,7 @@ public List getAllCoursesWithUserStats(@RequestParam(defaultValue = "fal * @param onlyActive if true, only active courses will be considered in the result * @return a list of courses (the user has access to) */ - @GetMapping("/courses/course-management-overview") + @GetMapping("courses/course-management-overview") @PreAuthorize("hasRole('TA')") public List getAllCoursesForManagementOverview(@RequestParam(defaultValue = "false") boolean onlyActive) { return courseService.getAllCoursesForManagementOverview(onlyActive); @@ -365,7 +358,7 @@ public List getAllCoursesForManagementOverview(@RequestParam(defaultValu * * @return the list of courses which are active */ - @GetMapping("/courses/for-registration") + @GetMapping("courses/for-registration") @PreAuthorize("hasRole('USER')") public List getAllCoursesToRegister() { log.debug("REST request to get all currently active Courses that are not online courses"); @@ -391,7 +384,7 @@ public List getAllCoursesToRegister() { * @param courseId the courseId for which exercises, lectures, exams and learning goals should be fetched * @return a course with all exercises, lectures, exams and learning goals visible to the student */ - @GetMapping("/courses/{courseId}/for-dashboard") + @GetMapping("courses/{courseId}/for-dashboard") @PreAuthorize("hasRole('USER')") public Course getCourseForDashboard(@PathVariable long courseId) { long start = System.currentTimeMillis(); @@ -407,7 +400,7 @@ public Course getCourseForDashboard(@PathVariable long courseId) { * * @return the list of courses (the user has access to) including all exercises with participation and result for the user */ - @GetMapping("/courses/for-dashboard") + @GetMapping("courses/for-dashboard") @PreAuthorize("hasRole('USER')") public List getAllCoursesForDashboard() { long start = System.currentTimeMillis(); @@ -425,7 +418,7 @@ public List getAllCoursesForDashboard() { * * @return the list of courses (the user has access to) */ - @GetMapping("/courses/for-notifications") + @GetMapping("courses/for-notifications") @PreAuthorize("hasRole('USER')") public List getAllCoursesForNotifications() { log.debug("REST request to get all Courses the user has access to"); @@ -439,7 +432,7 @@ public List getAllCoursesForNotifications() { * @param courseId the id of the course to retrieve * @return data about a course including all exercises, plus some data for the tutor as tutor status for assessment */ - @GetMapping("/courses/{courseId}/for-assessment-dashboard") + @GetMapping("courses/{courseId}/for-assessment-dashboard") @PreAuthorize("hasRole('TA')") public ResponseEntity getCourseForAssessmentDashboard(@PathVariable long courseId) { log.debug("REST request /courses/{courseId}/for-assessment-dashboard"); @@ -455,7 +448,8 @@ public ResponseEntity getCourseForAssessmentDashboard(@PathVariable long } /** - * GET /courses/:courseId/stats-for-assessment-dashboard A collection of useful statistics for the tutor course dashboard, including: - number of submissions to the course - number of + * GET /courses/:courseId/stats-for-assessment-dashboard A collection of useful statistics for the tutor course dashboard, including: - number of submissions to the course - + * number of * assessments - number of assessments assessed by the tutor - number of complaints *

* all timestamps were measured when calling this method from the PGdP assessment-dashboard @@ -463,7 +457,7 @@ public ResponseEntity getCourseForAssessmentDashboard(@PathVariable long * @param courseId the id of the course to retrieve * @return data about a course including all exercises, plus some data for the tutor as tutor status for assessment */ - @GetMapping("/courses/{courseId}/stats-for-assessment-dashboard") + @GetMapping("courses/{courseId}/stats-for-assessment-dashboard") @PreAuthorize("hasRole('TA')") public ResponseEntity getStatsForAssessmentDashboard(@PathVariable long courseId) { Course course = courseRepository.findByIdElseThrow(courseId); @@ -478,7 +472,7 @@ public ResponseEntity getStatsForAssessmentDashboard(@Path * @param courseId the id of the course to retrieve * @return the ResponseEntity with status 200 (OK) and with body the course, or with status 404 (Not Found) */ - @GetMapping("/courses/{courseId}") + @GetMapping("courses/{courseId}") @PreAuthorize("hasRole('USER')") public ResponseEntity getCourse(@PathVariable Long courseId) { log.debug("REST request to get Course : {}", courseId); @@ -506,7 +500,7 @@ public ResponseEntity getCourse(@PathVariable Long courseId) { * @param courseId the id of the course to retrieve * @return the ResponseEntity with status 200 (OK) and with body the course, or with status 404 (Not Found) */ - @GetMapping("/courses/{courseId}/with-exercises") + @GetMapping("courses/{courseId}/with-exercises") @PreAuthorize("hasRole('TA')") public ResponseEntity getCourseWithExercises(@PathVariable Long courseId) { log.debug("REST request to get Course : {}", courseId); @@ -517,10 +511,11 @@ public ResponseEntity getCourseWithExercises(@PathVariable Long courseId /** * GET /courses/:courseId/with-organizations Get a course by id with eagerly loaded organizations + * * @param courseId the id of the course * @return the course with eagerly loaded organizations */ - @GetMapping("/courses/{courseId}/with-organizations") + @GetMapping("courses/{courseId}/with-organizations") @PreAuthorize("hasRole('TA')") public ResponseEntity getCourseWithOrganizations(@PathVariable Long courseId) { log.debug("REST request to get a course with its organizations : {}", courseId); @@ -535,7 +530,7 @@ public ResponseEntity getCourseWithOrganizations(@PathVariable Long cour * @param courseId the id of the course * @return the ResponseEntity with status 200 (OK) and with body the course, or with status 404 (Not Found) */ - @GetMapping("/courses/{courseId}/lockedSubmissions") + @GetMapping("courses/{courseId}/lockedSubmissions") @PreAuthorize("hasRole('TA')") public ResponseEntity> getLockedSubmissionsForCourse(@PathVariable Long courseId) { log.debug("REST request to get all locked submissions for course : {}", courseId); @@ -559,7 +554,7 @@ public ResponseEntity> getLockedSubmissionsForCourse(@PathVaria * @param onlyActive if true, only active courses will be considered in the result * @return ResponseEntity with status, containing a list of courses */ - @GetMapping("/courses/exercises-for-management-overview") + @GetMapping("courses/exercises-for-management-overview") @PreAuthorize("hasRole('TA')") public ResponseEntity> getExercisesForCourseOverview(@RequestParam(defaultValue = "false") boolean onlyActive) { final List courses = new ArrayList<>(); @@ -580,7 +575,7 @@ public ResponseEntity> getExercisesForCourseOverview(@RequestParam( * @param onlyActive if true, only active courses will be considered in the result * @return ResponseEntity with status, containing a list of CourseManagementOverviewStatisticsDTO */ - @GetMapping("/courses/stats-for-management-overview") + @GetMapping("courses/stats-for-management-overview") @PreAuthorize("hasRole('TA')") public ResponseEntity> getExerciseStatsForCourseOverview(@RequestParam(defaultValue = "false") boolean onlyActive) { final List courseDTOs = new ArrayList<>(); @@ -609,7 +604,7 @@ public ResponseEntity> getExerciseSt * @param courseId the id of the course to delete * @return the ResponseEntity with status 200 (OK) */ - @DeleteMapping("/courses/{courseId}") + @DeleteMapping("courses/{courseId}") @PreAuthorize("hasRole('ADMIN')") public ResponseEntity deleteCourse(@PathVariable long courseId) { log.info("REST request to delete Course : {}", courseId); @@ -633,7 +628,7 @@ public ResponseEntity deleteCourse(@PathVariable long courseId) { * @param courseId the id of the course * @return empty */ - @PutMapping("/courses/{courseId}/archive") + @PutMapping("courses/{courseId}/archive") @PreAuthorize("hasRole('INSTRUCTOR')") @FeatureToggle(Feature.Exports) public ResponseEntity archiveCourse(@PathVariable Long courseId) { @@ -665,7 +660,7 @@ public ResponseEntity archiveCourse(@PathVariable Long courseId) { * @return ResponseEntity with status */ @PreAuthorize("hasRole('INSTRUCTOR')") - @GetMapping("/courses/{courseId}/download-archive") + @GetMapping("courses/{courseId}/download-archive") public ResponseEntity downloadCourseArchive(@PathVariable Long courseId) throws FileNotFoundException { log.info("REST request to download archive of Course : {}", courseId); final Course course = courseRepository.findByIdElseThrow(courseId); @@ -688,7 +683,7 @@ public ResponseEntity downloadCourseArchive(@PathVariable Long courseI * @param courseId id of the course to clean up * @return ResponseEntity with status */ - @DeleteMapping("/courses/{courseId}/cleanup") + @DeleteMapping("courses/{courseId}/cleanup") @PreAuthorize("hasRole('INSTRUCTOR')") public ResponseEntity cleanup(@PathVariable Long courseId) { log.info("REST request to cleanup the Course : {}", courseId); @@ -708,7 +703,7 @@ public ResponseEntity cleanup(@PathVariable Long courseId) { * @param courseId the id of the course to get the categories from * @return the ResponseEntity with status 200 (OK) and the list of categories or with status 404 (Not Found) */ - @GetMapping(value = "/courses/{courseId}/categories") + @GetMapping("courses/{courseId}/categories") @PreAuthorize("hasRole('EDITOR')") public ResponseEntity> getCategoriesInCourse(@PathVariable Long courseId) { log.debug("REST request to get categories of Course : {}", courseId); @@ -723,7 +718,7 @@ public ResponseEntity> getCategoriesInCourse(@PathVariable Long cour * @param courseId the id of the course * @return list of users with status 200 (OK) */ - @GetMapping(value = "/courses/{courseId}/students") + @GetMapping("courses/{courseId}/students") @PreAuthorize("hasRole('INSTRUCTOR')") public ResponseEntity> getAllStudentsInCourse(@PathVariable Long courseId) { log.debug("REST request to get all students in course : {}", courseId); @@ -738,7 +733,7 @@ public ResponseEntity> getAllStudentsInCourse(@PathVariable Long cour * @param loginOrName the login or name by which to search users * @return the ResponseEntity with status 200 (OK) and with body all users */ - @GetMapping("/courses/{courseId}/students/search") + @GetMapping("courses/{courseId}/students/search") @PreAuthorize("hasRole('INSTRUCTOR')") public ResponseEntity> searchStudentsInCourse(@PathVariable Long courseId, @RequestParam("loginOrName") String loginOrName) { log.debug("REST request to search for students in course : {} with login or name : {}", courseId, loginOrName); @@ -758,7 +753,7 @@ public ResponseEntity> searchStudentsInCourse(@PathVariable Long c * @param courseId the id of the course * @return list of users with status 200 (OK) */ - @GetMapping(value = "/courses/{courseId}/tutors") + @GetMapping("courses/{courseId}/tutors") @PreAuthorize("hasRole('INSTRUCTOR')") public ResponseEntity> getAllTutorsInCourse(@PathVariable Long courseId) { log.debug("REST request to get all tutors in course : {}", courseId); @@ -772,7 +767,7 @@ public ResponseEntity> getAllTutorsInCourse(@PathVariable Long course * @param courseId the id of the course * @return list of users with status 200 (OK) */ - @GetMapping(value = "/courses/{courseId}/editors") + @GetMapping("courses/{courseId}/editors") @PreAuthorize("hasRole('INSTRUCTOR')") public ResponseEntity> getAllEditorsInCourse(@PathVariable Long courseId) { log.debug("REST request to get all editors in course : {}", courseId); @@ -786,7 +781,7 @@ public ResponseEntity> getAllEditorsInCourse(@PathVariable Long cours * @param courseId the id of the course * @return list of users with status 200 (OK) */ - @GetMapping(value = "/courses/{courseId}/instructors") + @GetMapping("courses/{courseId}/instructors") @PreAuthorize("hasRole('INSTRUCTOR')") public ResponseEntity> getAllInstructorsInCourse(@PathVariable Long courseId) { log.debug("REST request to get all instructors in course : {}", courseId); @@ -797,11 +792,11 @@ public ResponseEntity> getAllInstructorsInCourse(@PathVariable Long c /** * GET /courses/:courseId/search-users : search users for a given course within all groups. * - * @param courseId the id of the course for which to search users - * @param nameOfUser the name by which to search users + * @param courseId the id of the course for which to search users + * @param nameOfUser the name by which to search users * @return the ResponseEntity with status 200 (OK) and with body all users */ - @GetMapping("/courses/{courseId}/search-other-users") + @GetMapping("courses/{courseId}/search-other-users") @PreAuthorize("hasRole('USER')") public ResponseEntity> searchOtherUsersInCourse(@PathVariable long courseId, @RequestParam("nameOfUser") String nameOfUser) { Course course = courseRepository.findByIdElseThrow(courseId); @@ -821,7 +816,7 @@ public ResponseEntity> searchOtherUsersInCourse(@PathVariable long co * @param courseId the id of the course * @return the title of the course wrapped in an ResponseEntity or 404 Not Found if no course with that id exists */ - @GetMapping(value = "/courses/{courseId}/title") + @GetMapping("courses/{courseId}/title") @PreAuthorize("hasRole('USER')") @ResponseBody public ResponseEntity getCourseTitle(@PathVariable Long courseId) { @@ -836,7 +831,7 @@ public ResponseEntity getCourseTitle(@PathVariable Long courseId) { * @param studentLogin the login of the user who should get student access * @return empty ResponseEntity with status 200 (OK) or with status 404 (Not Found) */ - @PostMapping(value = "/courses/{courseId}/students/{studentLogin:" + Constants.LOGIN_REGEX + "}") + @PostMapping("courses/{courseId}/students/{studentLogin:" + Constants.LOGIN_REGEX + "}") @PreAuthorize("hasRole('INSTRUCTOR')") public ResponseEntity addStudentToCourse(@PathVariable Long courseId, @PathVariable String studentLogin) { log.debug("REST request to add {} as student to course : {}", studentLogin, courseId); @@ -851,7 +846,7 @@ public ResponseEntity addStudentToCourse(@PathVariable Long courseId, @Pat * @param tutorLogin the login of the user who should get tutor access * @return empty ResponseEntity with status 200 (OK) or with status 404 (Not Found) */ - @PostMapping(value = "/courses/{courseId}/tutors/{tutorLogin:" + Constants.LOGIN_REGEX + "}") + @PostMapping("courses/{courseId}/tutors/{tutorLogin:" + Constants.LOGIN_REGEX + "}") @PreAuthorize("hasRole('INSTRUCTOR')") public ResponseEntity addTutorToCourse(@PathVariable Long courseId, @PathVariable String tutorLogin) { log.debug("REST request to add {} as tutors to course : {}", tutorLogin, courseId); @@ -862,11 +857,11 @@ public ResponseEntity addTutorToCourse(@PathVariable Long courseId, @PathV /** * Post /courses/:courseId/editors/:editorLogin : Add the given user to the editors of the course so that the student can access the course administration * - * @param courseId the id of the course + * @param courseId the id of the course * @param editorLogin the login of the user who should get editor access * @return empty ResponseEntity with status 200 (OK) or with status 404 (Not Found) */ - @PostMapping(value = "/courses/{courseId}/editors/{editorLogin:" + Constants.LOGIN_REGEX + "}") + @PostMapping("courses/{courseId}/editors/{editorLogin:" + Constants.LOGIN_REGEX + "}") @PreAuthorize("hasRole('INSTRUCTOR')") public ResponseEntity addEditorToCourse(@PathVariable Long courseId, @PathVariable String editorLogin) { log.debug("REST request to add {} as editors to course : {}", editorLogin, courseId); @@ -882,7 +877,7 @@ public ResponseEntity addEditorToCourse(@PathVariable Long courseId, @Path * @param instructorLogin the login of the user who should get instructors access * @return empty ResponseEntity with status 200 (OK) or with status 404 (Not Found) */ - @PostMapping(value = "/courses/{courseId}/instructors/{instructorLogin:" + Constants.LOGIN_REGEX + "}") + @PostMapping("courses/{courseId}/instructors/{instructorLogin:" + Constants.LOGIN_REGEX + "}") @PreAuthorize("hasRole('INSTRUCTOR')") public ResponseEntity addInstructorToCourse(@PathVariable Long courseId, @PathVariable String instructorLogin) { log.debug("REST request to add {} as instructors to course : {}", instructorLogin, courseId); @@ -922,7 +917,7 @@ public ResponseEntity addUserToCourseGroup(String userLogin, User instruct * @param studentLogin the login of the user who should lose student access * @return empty ResponseEntity with status 200 (OK) or with status 404 (Not Found) */ - @DeleteMapping(value = "/courses/{courseId}/students/{studentLogin:" + Constants.LOGIN_REGEX + "}") + @DeleteMapping("courses/{courseId}/students/{studentLogin:" + Constants.LOGIN_REGEX + "}") @PreAuthorize("hasRole('INSTRUCTOR')") public ResponseEntity removeStudentFromCourse(@PathVariable Long courseId, @PathVariable String studentLogin) { log.debug("REST request to remove {} as student from course : {}", studentLogin, courseId); @@ -937,7 +932,7 @@ public ResponseEntity removeStudentFromCourse(@PathVariable Long courseId, * @param tutorLogin the login of the user who should lose student access * @return empty ResponseEntity with status 200 (OK) or with status 404 (Not Found) */ - @DeleteMapping(value = "/courses/{courseId}/tutors/{tutorLogin:" + Constants.LOGIN_REGEX + "}") + @DeleteMapping("courses/{courseId}/tutors/{tutorLogin:" + Constants.LOGIN_REGEX + "}") @PreAuthorize("hasRole('INSTRUCTOR')") public ResponseEntity removeTutorFromCourse(@PathVariable Long courseId, @PathVariable String tutorLogin) { log.debug("REST request to remove {} as tutor from course : {}", tutorLogin, courseId); @@ -948,11 +943,11 @@ public ResponseEntity removeTutorFromCourse(@PathVariable Long courseId, @ /** * DELETE /courses/:courseId/editors/:editorsLogin : Remove the given user from the editors of the course so that the editors cannot access the course administration anymore * - * @param courseId the id of the course + * @param courseId the id of the course * @param editorLogin the login of the user who should lose student access * @return empty ResponseEntity with status 200 (OK) or with status 404 (Not Found) */ - @DeleteMapping(value = "/courses/{courseId}/editors/{editorLogin:" + Constants.LOGIN_REGEX + "}") + @DeleteMapping("courses/{courseId}/editors/{editorLogin:" + Constants.LOGIN_REGEX + "}") @PreAuthorize("hasRole('INSTRUCTOR')") public ResponseEntity removeEditorFromCourse(@PathVariable Long courseId, @PathVariable String editorLogin) { log.debug("REST request to remove {} as editor from course : {}", editorLogin, courseId); @@ -961,13 +956,14 @@ public ResponseEntity removeEditorFromCourse(@PathVariable Long courseId, } /** - * DELETE /courses/:courseId/instructors/:instructorLogin : Remove the given user from the instructors of the course so that the instructor cannot access the course administration anymore + * DELETE /courses/:courseId/instructors/:instructorLogin : Remove the given user from the instructors of the course so that the instructor cannot access the course + * administration anymore * * @param courseId the id of the course * @param instructorLogin the login of the user who should lose student access * @return empty ResponseEntity with status 200 (OK) or with status 404 (Not Found) */ - @DeleteMapping(value = "/courses/{courseId}/instructors/{instructorLogin:" + Constants.LOGIN_REGEX + "}") + @DeleteMapping("courses/{courseId}/instructors/{instructorLogin:" + Constants.LOGIN_REGEX + "}") @PreAuthorize("hasRole('INSTRUCTOR')") public ResponseEntity removeInstructorFromCourse(@PathVariable Long courseId, @PathVariable String instructorLogin) { log.debug("REST request to remove {} as instructor from course : {}", instructorLogin, courseId); @@ -1004,7 +1000,7 @@ public ResponseEntity removeUserFromCourseGroup(String userLogin, User ins * @param courseId the id of the course * @return the ResponseEntity with status 200 (OK) and the body, or with status 404 (Not Found) */ - @GetMapping("/courses/{courseId}/management-detail") + @GetMapping("courses/{courseId}/management-detail") @PreAuthorize("hasRole('TA')") public ResponseEntity getCourseDTOForDetailView(@PathVariable Long courseId) { Course course = courseRepository.findByIdElseThrow(courseId); @@ -1016,7 +1012,7 @@ public ResponseEntity getCourseDTOForDetailView(@ /** * GET /courses/:courseId/statistics : Get the active students for this particular course * - * @param courseId the id of the course + * @param courseId the id of the course * @param periodIndex an index indicating which time period, 0 is current week, -1 is one week in the past, -2 is two weeks in the past ... * @return the ResponseEntity with status 200 (OK) and the data in body, or status 404 (Not Found) */ @@ -1063,9 +1059,9 @@ public ResponseEntity> getActiveStudentsForCourseLiveTime(@PathVar * This method first tries to find the student in the internal Artemis user database (because the user is most probably already using Artemis). * In case the user cannot be found, we additionally search the (TUM) LDAP in case it is configured properly. * - * @param courseId the id of the course - * @param studentDtos the list of students (with at least registration number) who should get access to the course - * @param courseGroup the group, the user has to be added to, either 'students', 'tutors', 'instructors' or 'editors' + * @param courseId the id of the course + * @param studentDtos the list of students (with at least registration number) who should get access to the course + * @param courseGroup the group, the user has to be added to, either 'students', 'tutors', 'instructors' or 'editors' * @return the list of students who could not be registered for the course, because they could NOT be found in the Artemis database and could NOT be found in the TUM LDAP */ @PostMapping("courses/{courseId}/{courseGroup}") diff --git a/src/main/webapp/app/course/manage/course-management.service.ts b/src/main/webapp/app/course/manage/course-management.service.ts index ce3c83af27c5..797ff340ebf2 100644 --- a/src/main/webapp/app/course/manage/course-management.service.ts +++ b/src/main/webapp/app/course/manage/course-management.service.ts @@ -19,6 +19,7 @@ import { CourseManagementDetailViewDto } from 'app/course/manage/course-manageme import { StudentDTO } from 'app/entities/student-dto.model'; import { EntityTitleService, EntityType } from 'app/shared/layouts/navbar/entity-title.service'; import { convertDateFromClient } from 'app/utils/date.utils'; +import { objectToJsonBlob } from 'app/utils/blob-util'; export type EntityResponseType = HttpResponse; export type EntityArrayResponseType = HttpResponse; @@ -37,19 +38,37 @@ export class CourseManagementService { /** * creates a course using a POST request * @param course - the course to be created on the server + * @param courseImage - the course icon file */ - create(course: Course): Observable { + create(course: Course, courseImage?: Blob): Observable { const copy = CourseManagementService.convertCourseDatesFromClient(course); - return this.http.post(this.resourceUrl, copy, { observe: 'response' }).pipe(map((res: EntityResponseType) => this.processCourseEntityResponseType(res))); + const formData = new FormData(); + formData.append('course', objectToJsonBlob(copy)); + if (courseImage) { + // The image was cropped by us and is a blob, so we need to set a placeholder name for the server check + formData.append('file', courseImage, 'placeholderName.png'); + } + + return this.http.post(this.resourceUrl, formData, { observe: 'response' }).pipe(map((res: EntityResponseType) => this.processCourseEntityResponseType(res))); } /** * updates a course using a PUT request - * @param course - the course to be updated - */ - update(course: Course): Observable { - const copy = CourseManagementService.convertCourseDatesFromClient(course); - return this.http.put(this.resourceUrl, copy, { observe: 'response' }).pipe(map((res: EntityResponseType) => this.processCourseEntityResponseType(res))); + * @param courseId - the id of the course to be updated + * @param courseUpdate - the updates to the course + * @param courseImage - the course icon file + */ + update(courseId: number, courseUpdate: Course, courseImage?: Blob): Observable { + const copy = CourseManagementService.convertCourseDatesFromClient(courseUpdate); + const formData = new FormData(); + formData.append('course', objectToJsonBlob(copy)); + if (courseImage) { + // The image was cropped by us and is a blob, so we need to set a placeholder name for the server check + formData.append('file', courseImage, 'placeholderName.png'); + } + return this.http + .put(`${this.resourceUrl}/${courseId}`, formData, { observe: 'response' }) + .pipe(map((res: EntityResponseType) => this.processCourseEntityResponseType(res))); } /** diff --git a/src/main/webapp/app/course/manage/course-update.component.html b/src/main/webapp/app/course/manage/course-update.component.html index 9a0862ecf015..73af6cf35d2b 100644 --- a/src/main/webapp/app/course/manage/course-update.component.html +++ b/src/main/webapp/app/course/manage/course-update.component.html @@ -67,12 +67,6 @@

-
- -
diff --git a/src/main/webapp/app/course/manage/course-update.component.ts b/src/main/webapp/app/course/manage/course-update.component.ts index d69d661f9815..d7a9ec78bf3a 100644 --- a/src/main/webapp/app/course/manage/course-update.component.ts +++ b/src/main/webapp/app/course/manage/course-update.component.ts @@ -221,9 +221,9 @@ export class CourseUpdateComponent implements OnInit { course.onlineCourseConfiguration = this.isOnlineCourse() ? this.onlineCourseConfigurationForm.getRawValue() : null; if (this.course.id !== undefined) { - this.subscribeToSaveResponse(this.courseService.update(course)); + this.subscribeToSaveResponse(this.courseService.update(this.course.id, course, file)); } else { - this.subscribeToSaveResponse(this.courseService.create(course)); + this.subscribeToSaveResponse(this.courseService.create(course, file)); } } @@ -278,33 +278,6 @@ export class CourseUpdateComponent implements OnInit { this.showCropper = true; } - /** - * @function uploadBackground - * @desc Upload the selected file (from "Upload Background") and use it for the question's backgroundFilePath - */ - uploadCourseImage(): void { - const contentType = 'image/*'; - const base64Data = this.croppedImage.replace('data:image/png;base64,', ''); - const file = base64StringToBlob(base64Data, contentType); - const fileName = this.courseImageFileName; - - this.isUploadingCourseImage = true; - this.fileUploaderService.uploadFile(file, fileName).then( - (response) => { - this.courseForm.patchValue({ courseIcon: response.path }); - this.isUploadingCourseImage = false; - this.courseImageFile = undefined; - this.courseImageFileName = response.path!; - }, - () => { - this.isUploadingCourseImage = false; - this.courseImageFile = undefined; - this.courseImageFileName = this.course.courseIcon!; - }, - ); - this.showCropper = false; - } - /** * Action on unsuccessful course creation or edit * @param error The error for providing feedback diff --git a/src/main/webapp/app/shared/http/file-uploader.service.ts b/src/main/webapp/app/shared/http/file-uploader.service.ts index fe25b5850be4..4965d1e2335e 100644 --- a/src/main/webapp/app/shared/http/file-uploader.service.ts +++ b/src/main/webapp/app/shared/http/file-uploader.service.ts @@ -55,7 +55,7 @@ export class FileUploaderService { return Promise.reject(new Error('File is too big! Maximum allowed file size: ' + MAX_FILE_SIZE / (1024 * 1024) + ' MB.')); } - const keepFileName: boolean = !!options && options.keepFileName; + const keepFileName: boolean = !!options?.keepFileName; const formData = new FormData(); formData.append('file', file, fileName); return lastValueFrom(this.http.post(endpoint + `?keepFileName=${keepFileName}`, formData)); diff --git a/src/test/javascript/spec/component/course/course-management.service.spec.ts b/src/test/javascript/spec/component/course/course-management.service.spec.ts index 8295d7ae530f..fb4326e243df 100644 --- a/src/test/javascript/spec/component/course/course-management.service.spec.ts +++ b/src/test/javascript/spec/component/course/course-management.service.spec.ts @@ -108,11 +108,11 @@ describe('Course Management Service', () => { it('should update course', fakeAsync(() => { courseManagementService - .update({ ...course }) + .update(1, { ...course }) .pipe(take(1)) .subscribe((res) => expect(res.body).toEqual(course)); - const req = httpMock.expectOne({ method: 'PUT', url: resourceUrl }); + const req = httpMock.expectOne({ method: 'PUT', url: `${resourceUrl}/1` }); req.flush(returnedFromService); tick(); })); diff --git a/src/test/javascript/spec/component/course/course-update.component.spec.ts b/src/test/javascript/spec/component/course/course-update.component.spec.ts index 7c14e2e03266..6821fe33e13b 100644 --- a/src/test/javascript/spec/component/course/course-update.component.spec.ts +++ b/src/test/javascript/spec/component/course/course-update.component.spec.ts @@ -272,35 +272,6 @@ describe('Course Management Update Component', () => { }); }); - describe('uploadCourseImage', () => { - let croppedImage: string; - beforeEach(() => { - croppedImage = 'testCroppedImage'; - comp.croppedImage = 'data:image/png;base64,' + comp.croppedImage; - comp.courseImageFileName = 'testFilename'; - comp.showCropper = true; - comp.ngOnInit(); - }); - it('should upload new image and update form', () => { - uploadStub.mockResolvedValue({ path: 'testPath' } as FileUploadResponse); - comp.uploadCourseImage(); - const file = base64StringToBlob(croppedImage, 'image/*'); - // @ts-ignore - file['name'] = comp.courseImageFileName; - expect(uploadStub.mock.calls[0][1]).toBe(comp.courseImageFileName); - expect(comp.showCropper).toBeFalse(); - }); - it('should set image name to course icon if upload fails', () => { - uploadStub.mockRejectedValue({} as FileUploadResponse); - comp.course = new Course(); - comp.course.courseIcon = 'testCourseIcon'; - comp.uploadCourseImage(); - expect(uploadStub.mock.calls[0][1]).toBe(comp.courseImageFileName); - expect(comp.courseImageFileName).toBe(comp.course.courseIcon); - expect(comp.showCropper).toBeFalse(); - }); - }); - describe('changePresentationScoreInput', () => { it('should enabled if control is disabled', () => { const control = new FormControl(12); diff --git a/src/test/javascript/spec/service/course.service.spec.ts b/src/test/javascript/spec/service/course.service.spec.ts index 687333c334c8..3096fea946e0 100644 --- a/src/test/javascript/spec/service/course.service.spec.ts +++ b/src/test/javascript/spec/service/course.service.spec.ts @@ -109,7 +109,7 @@ describe('Course Service', () => { returnedFromService, ); service - .update(expected) + .update(1, expected) .pipe(take(1)) .subscribe((resp) => expect(resp).toMatchObject({ body: expected })); const req = httpMock.expectOne({ method: 'PUT' }); From 1c191455830502fae6ab51ef1bca780fb4be6be2 Mon Sep 17 00:00:00 2001 From: Julian Christl Date: Thu, 13 Oct 2022 14:52:01 +0200 Subject: [PATCH 02/16] fix cr comments --- .../in/www1/artemis/web/rest/CourseResource.java | 14 +++++++++----- .../course/course-update.component.spec.ts | 10 ++-------- 2 files changed, 11 insertions(+), 13 deletions(-) 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 ffa8928ebbfc..23c46274ce52 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 @@ -122,6 +122,7 @@ public CourseResource(UserRepository userRepository, CourseService courseService * POST /courses : create a new course. * * @param course the course to create + * @param file the optional course icon file * @return the ResponseEntity with status 201 (Created) and with body the new course, or with status 400 (Bad Request) if the course has already an ID * @throws URISyntaxException if the Location URI syntax is incorrect */ @@ -163,18 +164,20 @@ public ResponseEntity createCourse(@RequestPart Course course, @RequestP } /** - * PUT /courses/:id : Updates an existing updatedCourse. + * PUT /courses/:courseId : Updates an existing updatedCourse. * + * @param courseId the id of the course to update * @param courseUpdate the course to update + * @param file the optional course icon file * @return the ResponseEntity with status 200 (OK) and with body the updated course */ - @PutMapping(value = "courses/{id}", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) + @PutMapping(value = "courses/{courseId}", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) @PreAuthorize("hasRole('INSTRUCTOR')") - public ResponseEntity updateCourse(@PathVariable Long id, @RequestPart("course") Course courseUpdate, @RequestPart(required = false) MultipartFile file) { + public ResponseEntity updateCourse(@PathVariable Long courseId, @RequestPart("course") Course courseUpdate, @RequestPart(required = false) MultipartFile file) { log.debug("REST request to update Course : {}", courseUpdate); User user = userRepository.getUserWithGroupsAndAuthorities(); - var existingCourse = courseRepository.findByIdWithOrganizationsAndLearningGoalsElseThrow(id); + var existingCourse = courseRepository.findByIdWithOrganizationsAndLearningGoalsElseThrow(courseId); if (!Objects.equals(existingCourse.getShortName(), courseUpdate.getShortName())) { throw new BadRequestAlertException("The course short name cannot be changed", Course.ENTITY_NAME, "shortNameCannotChange", true); } @@ -226,6 +229,7 @@ public ResponseEntity updateCourse(@PathVariable Long id, @RequestPart(" courseUpdate.setCourseIcon(pathString); } + courseUpdate.setId(courseId); // Don't persist a wrong ID Course result = courseRepository.save(courseUpdate); // Based on the old instructors, editors and TAs, we can update all exercises in the course in the VCS (if necessary) @@ -271,7 +275,7 @@ public ResponseEntity registerForCourse(@PathVariable Long courseId) { } if (!Boolean.TRUE.equals(course.isRegistrationEnabled())) { return ResponseEntity.badRequest().headers(HeaderUtil.createFailureAlert(applicationName, false, Course.ENTITY_NAME, "registrationDisabled", - "The course does not allow registration. Cannot register " + "user")).body(null); + "The course does not allow registration. Cannot register user")).body(null); } if (course.getOrganizations() != null && !course.getOrganizations().isEmpty() && !courseRepository.checkIfUserIsMemberOfCourseOrganizations(user, course)) { return ResponseEntity.badRequest().headers(HeaderUtil.createFailureAlert(applicationName, false, Course.ENTITY_NAME, "registrationNotAllowed", diff --git a/src/test/javascript/spec/component/course/course-update.component.spec.ts b/src/test/javascript/spec/component/course/course-update.component.spec.ts index 6821fe33e13b..c6b75b8ac8e9 100644 --- a/src/test/javascript/spec/component/course/course-update.component.spec.ts +++ b/src/test/javascript/spec/component/course/course-update.component.spec.ts @@ -25,9 +25,7 @@ import { ProfileInfo } from 'app/shared/layouts/profiles/profile-info.model'; import { OrganizationManagementService } from 'app/admin/organization-management/organization-management.service'; import { Organization } from 'app/entities/organization.model'; import dayjs from 'dayjs/esm'; -import { FileUploaderService, FileUploadResponse } from 'app/shared/http/file-uploader.service'; import { ImageCropperModule } from 'app/shared/image-cropper/image-cropper.module'; -import { base64StringToBlob } from 'app/utils/blob-util'; import { ProgrammingLanguage } from 'app/entities/programming-exercise.model'; @Component({ selector: 'jhi-markdown-editor', template: '' }) @@ -44,8 +42,6 @@ describe('Course Management Update Component', () => { let profileService: ProfileService; let organizationService: OrganizationManagementService; let course: Course; - let fileUploaderService: FileUploaderService; - let uploadStub: jest.SpyInstance; beforeEach(() => { course = new Course(); @@ -109,8 +105,6 @@ describe('Course Management Update Component', () => { service = TestBed.inject(CourseManagementService); profileService = TestBed.inject(ProfileService); organizationService = TestBed.inject(OrganizationManagementService); - fileUploaderService = TestBed.inject(FileUploaderService); - uploadStub = jest.spyOn(fileUploaderService, 'uploadFile'); }); }); @@ -201,7 +195,7 @@ describe('Course Management Update Component', () => { // THEN expect(updateStub).toHaveBeenCalledOnce(); - expect(updateStub).toHaveBeenCalledWith({ ...entity, onlineCourseConfiguration: null }); + expect(updateStub).toHaveBeenCalledWith(entity.id, { ...entity, onlineCourseConfiguration: null }, undefined); expect(comp.isSaving).toBeFalse(); })); @@ -234,7 +228,7 @@ describe('Course Management Update Component', () => { // THEN expect(createStub).toHaveBeenCalledOnce(); - expect(createStub).toHaveBeenCalledWith({ ...entity, onlineCourseConfiguration: null }); + expect(createStub).toHaveBeenCalledWith({ ...entity, onlineCourseConfiguration: null }, undefined); expect(comp.isSaving).toBeFalse(); })); }); From 43c74fa4f316557e9a1bf5b4950649f116c67c7f Mon Sep 17 00:00:00 2001 From: Julian Christl Date: Thu, 13 Oct 2022 15:34:08 +0200 Subject: [PATCH 03/16] improve inputs of image cropper --- .../component/image-cropper.component.ts | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/main/webapp/app/shared/image-cropper/component/image-cropper.component.ts b/src/main/webapp/app/shared/image-cropper/component/image-cropper.component.ts index c5fee4f0b1de..edc741420fc9 100644 --- a/src/main/webapp/app/shared/image-cropper/component/image-cropper.component.ts +++ b/src/main/webapp/app/shared/image-cropper/component/image-cropper.component.ts @@ -28,6 +28,7 @@ import { CropperPosition } from 'app/shared/image-cropper/interfaces/cropper-pos import { ImageCroppedEvent } from 'app/shared/image-cropper/interfaces/image-cropped-event.interface'; // Note: this component and all files in the image-cropper folder were taken from https://github.com/Mawi137/ngx-image-cropper because the framework was not maintained anymore +// Note: Partially adapted to fit Artemis needs @Component({ // tslint:disable-next-line:component-selector @@ -52,10 +53,10 @@ export class ImageCropperComponent implements OnChanges, OnInit { @ViewChild('wrapper', { static: true }) wrapper: ElementRef; @ViewChild('sourceImage', { static: false }) sourceImage: ElementRef; - @Input() imageChangedEvent: any; - @Input() imageURL: string; - @Input() imageBase64: string; - @Input() imageFile: File; + @Input() imageChangedEvent?: Event; + @Input() imageURL?: string; + @Input() imageBase64?: string; + @Input() imageFile?: File; @Input() format: OutputFormat = this.settings.format; @Input() transform: ImageTransform = {}; @@ -156,7 +157,9 @@ export class ImageCropperComponent implements OnChanges, OnInit { this.reset(); } if (changes.imageChangedEvent && this.isValidImageChangedEvent()) { - this.loadImageFile(this.imageChangedEvent.target.files[0]); + const element = this.imageChangedEvent?.currentTarget as HTMLInputElement; + // @ts-ignore Ignore "is possibly null", checked before + this.loadImageFile(element.files[0]); } if (changes.imageURL && this.imageURL) { this.loadImageFromURL(this.imageURL); @@ -170,7 +173,12 @@ export class ImageCropperComponent implements OnChanges, OnInit { } private isValidImageChangedEvent(): boolean { - return this.imageChangedEvent?.target?.files?.length > 0; + if (!this.imageChangedEvent?.currentTarget) { + return false; + } + const element = this.imageChangedEvent.currentTarget as HTMLInputElement; + + return !!element.files && element.files.length > 0; } private setCssTransform() { From 46e7d8acf4915f9aa2285b519cf248ba841cc86b Mon Sep 17 00:00:00 2001 From: Julian Christl Date: Thu, 13 Oct 2022 15:35:09 +0200 Subject: [PATCH 04/16] adapt component to have less properties, fix save issue --- .../manage/course-update.component.html | 2 +- .../course/manage/course-update.component.ts | 25 +++++++++---------- .../course/course-update.component.spec.ts | 6 ++--- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/main/webapp/app/course/manage/course-update.component.html b/src/main/webapp/app/course/manage/course-update.component.html index 73af6cf35d2b..20b30feaa6db 100644 --- a/src/main/webapp/app/course/manage/course-update.component.html +++ b/src/main/webapp/app/course/manage/course-update.component.html @@ -55,7 +55,7 @@

{ length: 1, item: () => file, } as unknown as FileList; - const event = { target: { files: fileList } }; + const event = { target: { files: fileList } } as unknown as Event; comp.setCourseImage(event); - expect(comp.courseImageFile).toEqual(file); - expect(comp.courseImageFileName).toBe('testFilename'); - expect(comp.imageChangedEvent).toBe(event); + expect(comp.courseImageUploadFile).toEqual(file); }); }); From 7a5018201e5347ad7be40d22749a734c849c6545 Mon Sep 17 00:00:00 2001 From: Julian Christl Date: Tue, 18 Oct 2022 20:48:21 +0200 Subject: [PATCH 05/16] adapt server tests --- ...rseBitbucketBambooJiraIntegrationTest.java | 18 -- .../CourseGitlabJenkinsIntegrationTest.java | 42 ++--- .../www1/artemis/util/CourseTestService.java | 174 ++++++++++-------- 3 files changed, 117 insertions(+), 117 deletions(-) diff --git a/src/test/java/de/tum/in/www1/artemis/CourseBitbucketBambooJiraIntegrationTest.java b/src/test/java/de/tum/in/www1/artemis/CourseBitbucketBambooJiraIntegrationTest.java index e621dd221699..bcbeb65ddf66 100644 --- a/src/test/java/de/tum/in/www1/artemis/CourseBitbucketBambooJiraIntegrationTest.java +++ b/src/test/java/de/tum/in/www1/artemis/CourseBitbucketBambooJiraIntegrationTest.java @@ -131,24 +131,6 @@ void testCreateCourseWithWrongShortName() throws Exception { courseTestService.testCreateCourseWithWrongShortName(); } - @Test - @WithMockUser(username = "admin", roles = "ADMIN") - void testUpdateCourseWithWrongShortName() throws Exception { - courseTestService.testUpdateCourseWithWrongShortName(); - } - - @Test - @WithMockUser(username = "admin", roles = "ADMIN") - void testUpdateCourseWithoutId() throws Exception { - courseTestService.testUpdateCourseWithoutId(); - } - - @Test - @WithMockUser(username = "instructor1", roles = "INSTRUCTOR") - void testUpdateCourseWithoutIdAsInstructor() throws Exception { - courseTestService.testUpdateCourseWithoutIdAsInstructor(); - } - @Test @WithMockUser(username = "admin", roles = "ADMIN") void testUpdateCourseIsEmpty() throws Exception { diff --git a/src/test/java/de/tum/in/www1/artemis/CourseGitlabJenkinsIntegrationTest.java b/src/test/java/de/tum/in/www1/artemis/CourseGitlabJenkinsIntegrationTest.java index 6ff53e52ccdf..877889798d33 100644 --- a/src/test/java/de/tum/in/www1/artemis/CourseGitlabJenkinsIntegrationTest.java +++ b/src/test/java/de/tum/in/www1/artemis/CourseGitlabJenkinsIntegrationTest.java @@ -2,6 +2,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.verifyNoInteractions; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import java.util.HashSet; import java.util.Optional; @@ -14,6 +15,9 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; import org.springframework.security.test.context.support.WithMockUser; +import org.springframework.test.web.servlet.MvcResult; + +import com.fasterxml.jackson.databind.ObjectMapper; import de.tum.in.www1.artemis.domain.Course; import de.tum.in.www1.artemis.domain.User; @@ -37,6 +41,9 @@ class CourseGitlabJenkinsIntegrationTest extends AbstractSpringIntegrationJenkin @Autowired private UserRepository userRepo; + @Autowired + private ObjectMapper objectMapper; + @BeforeEach void setup() { courseTestService.setup(this); @@ -133,24 +140,6 @@ void testCreateCourseWithWrongShortName() throws Exception { courseTestService.testCreateCourseWithWrongShortName(); } - @Test - @WithMockUser(username = "admin", roles = "ADMIN") - void testUpdateCourseWithWrongShortName() throws Exception { - courseTestService.testUpdateCourseWithWrongShortName(); - } - - @Test - @WithMockUser(username = "admin", roles = "ADMIN") - void testUpdateCourseWithoutId() throws Exception { - courseTestService.testUpdateCourseWithoutId(); - } - - @Test - @WithMockUser(username = "instructor1", roles = "INSTRUCTOR") - void testUpdateCourseWithoutIdAsInstructor() throws Exception { - courseTestService.testUpdateCourseWithoutIdAsInstructor(); - } - @Test @WithMockUser(username = "admin", roles = "ADMIN") void testUpdateCourseIsEmpty() throws Exception { @@ -188,7 +177,9 @@ void testUpdateOldMembersInCourse() throws Exception { gitlabRequestMockProvider.mockUpdateCoursePermissions(course, oldInstructorGroup, course.getEditorGroupName(), course.getTeachingAssistantGroupName()); jenkinsRequestMockProvider.mockUpdateCoursePermissions(course, oldInstructorGroup, course.getEditorGroupName(), course.getTeachingAssistantGroupName(), false, false); - course = request.putWithResponseBody("/api/courses", course, Course.class, HttpStatus.OK); + + MvcResult result = request.getMvc().perform(courseTestService.buildUpdateCourse(course.getId(), course)).andExpect(status().isOk()).andReturn(); + course = objectMapper.readValue(result.getResponse().getContentAsString(), Course.class); assertThat(course.getInstructorGroupName()).isEqualTo("new-editor-group"); } @@ -220,7 +211,8 @@ void testSetPermissionsForNewGroupMembersInCourse() throws Exception { gitlabRequestMockProvider.mockUpdateCoursePermissions(course, oldInstructorGroup, oldEditorGroup, oldTaGroup); jenkinsRequestMockProvider.mockUpdateCoursePermissions(course, oldInstructorGroup, course.getEditorGroupName(), course.getTeachingAssistantGroupName(), false, false); - course = request.putWithResponseBody("/api/courses", course, Course.class, HttpStatus.OK); + MvcResult result = request.getMvc().perform(courseTestService.buildUpdateCourse(course.getId(), course)).andExpect(status().isOk()).andReturn(); + course = objectMapper.readValue(result.getResponse().getContentAsString(), Course.class); assertThat(course.getInstructorGroupName()).isEqualTo("new-instructor-group"); assertThat(course.getEditorGroupName()).isEqualTo("new-editor-group"); @@ -239,7 +231,7 @@ void testSetPermissionsForNewGroupMembersInCourseFails() throws Exception { userRepo.save(user); gitlabRequestMockProvider.mockGetUserId(user.getLogin(), true, true); - request.putWithResponseBody("/api/courses", course, Course.class, HttpStatus.INTERNAL_SERVER_ERROR); + request.getMvc().perform(courseTestService.buildUpdateCourse(course.getId(), course)).andExpect(status().isInternalServerError()).andReturn(); } @Test @@ -254,7 +246,7 @@ void testSetPermissionsForNewGroupMembersInCourseMemberDoesntExist() throws Exce userRepo.save(user); gitlabRequestMockProvider.mockFailOnGetUserById(user.getLogin()); - request.putWithResponseBody("/api/courses", course, Course.class, HttpStatus.INTERNAL_SERVER_ERROR); + request.getMvc().perform(courseTestService.buildUpdateCourse(course.getId(), course)).andExpect(status().isInternalServerError()).andReturn(); } @Test @@ -273,7 +265,7 @@ void testFailToUpdateOldMembersInCourse() throws Exception { assertThat(user).isPresent(); gitlabRequestMockProvider.mockFailToUpdateOldGroupMembers(exercise.get(), user.get()); - request.putWithResponseBody("/api/courses", course, Course.class, HttpStatus.INTERNAL_SERVER_ERROR); + request.getMvc().perform(courseTestService.buildUpdateCourse(course.getId(), course)).andExpect(status().isInternalServerError()); } /** @@ -302,7 +294,7 @@ void testUpdateCourseGroupsFailsToGetUser() throws Exception { assertThat(user).isPresent(); gitlabRequestMockProvider.mockFailToGetUserWhenUpdatingOldMembers(user.get()); - request.putWithResponseBody("/api/courses", course, Course.class, HttpStatus.INTERNAL_SERVER_ERROR); + request.getMvc().perform(courseTestService.buildUpdateCourse(course.getId(), course)).andExpect(status().isInternalServerError()).andReturn(); } @Test @@ -318,7 +310,7 @@ void testUpdateCourseGroupsFailsToRemoveOldMember() throws Exception { assertThat(exercise).isPresent(); gitlabRequestMockProvider.mockFailToRemoveOldMember(exercise.get(), user.get()); - request.putWithResponseBody("/api/courses", course, Course.class, HttpStatus.INTERNAL_SERVER_ERROR); + request.getMvc().perform(courseTestService.buildUpdateCourse(course.getId(), course)).andExpect(status().isInternalServerError()).andReturn(); } @Test diff --git a/src/test/java/de/tum/in/www1/artemis/util/CourseTestService.java b/src/test/java/de/tum/in/www1/artemis/util/CourseTestService.java index 16aef4f73891..6c50c6846e54 100644 --- a/src/test/java/de/tum/in/www1/artemis/util/CourseTestService.java +++ b/src/test/java/de/tum/in/www1/artemis/util/CourseTestService.java @@ -5,6 +5,7 @@ import static org.awaitility.Awaitility.await; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.*; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import java.io.IOException; import java.nio.file.Files; @@ -15,16 +16,27 @@ import java.util.*; import java.util.stream.Collectors; +import javax.validation.constraints.NotNull; + import org.mockito.ArgumentMatchers; import org.mockito.MockedStatic; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.actuate.audit.AuditEvent; +import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; +import org.springframework.mock.web.MockMultipartFile; import org.springframework.stereotype.Service; +import org.springframework.test.web.servlet.MvcResult; +import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; + import de.tum.in.www1.artemis.config.Constants; import de.tum.in.www1.artemis.domain.*; import de.tum.in.www1.artemis.domain.enumeration.*; @@ -129,6 +141,9 @@ public class CourseTestService { @Autowired private OnlineCourseConfigurationRepository onlineCourseConfigurationRepository; + @Autowired + private ObjectMapper objectMapper; + private final static int numberOfStudents = 8; private final static int numberOfTutors = 5; @@ -167,12 +182,12 @@ public void testCreateCourseWithPermission() throws Exception { mockDelegate.mockCreateGroupInUserManagement(course.getDefaultEditorGroupName()); mockDelegate.mockCreateGroupInUserManagement(course.getDefaultInstructorGroupName()); - request.post("/api/courses", course, HttpStatus.CREATED); + request.getMvc().perform(buildCreateCourse(course)).andExpect(status().isCreated()); List repoContent = courseRepo.findAll(); assertThat(repoContent).as("Course got stored").hasSize(1); course = ModelFactory.generateCourse(1L, null, null, new HashSet<>()); - request.post("/api/courses", course, HttpStatus.BAD_REQUEST); + request.getMvc().perform(buildCreateCourse(course)).andExpect(status().isBadRequest()); assertThat(courseRepo.findAll()).as("Course has not been stored").contains(repoContent.toArray(new Course[0])); } @@ -185,12 +200,12 @@ public void testCreateCourseWithSameShortName() throws Exception { mockDelegate.mockCreateGroupInUserManagement(course1.getDefaultEditorGroupName()); mockDelegate.mockCreateGroupInUserManagement(course1.getDefaultInstructorGroupName()); - request.post("/api/courses", course1, HttpStatus.CREATED); + request.getMvc().perform(buildCreateCourse(course1)).andExpect(status().isCreated()); assertThat(courseRepo.findAll()).as("Course got stored").hasSize(1); Course course2 = ModelFactory.generateCourse(null, null, null, new HashSet<>(), "tumuser", "tutor", "editor", "instructor"); course2.setShortName("shortName"); - request.post("/api/courses", course2, HttpStatus.BAD_REQUEST); + request.getMvc().perform(buildCreateCourse(course2)).andExpect(status().isBadRequest()); assertThat(courseRepo.findAll()).as("Course has not been stored").hasSize(1); } @@ -200,7 +215,9 @@ private void testCreateCourseWithNegativeValue(Course course) throws Exception { mockDelegate.mockCreateGroupInUserManagement(course.getDefaultTeachingAssistantGroupName()); mockDelegate.mockCreateGroupInUserManagement(course.getDefaultEditorGroupName()); mockDelegate.mockCreateGroupInUserManagement(course.getDefaultInstructorGroupName()); - request.post("/api/courses", course, HttpStatus.BAD_REQUEST); + var coursePart = new MockMultipartFile("course", "", MediaType.APPLICATION_JSON_VALUE, objectMapper.writeValueAsString(course).getBytes()); + var builder = MockMvcRequestBuilders.multipart(HttpMethod.POST, "/api/courses").file(coursePart).contentType(MediaType.MULTIPART_FORM_DATA_VALUE); + request.getMvc().perform(builder).andExpect(status().isBadRequest()); List repoContent = courseRepo.findAll(); assertThat(repoContent).as("Course has not been stored").isEmpty(); } @@ -252,21 +269,21 @@ public void testCreateCourseWithModifiedMaxComplainTimeDaysAndMaxComplains() thr course.setMaxComplaints(1); course.setMaxTeamComplaints(0); course.setMaxRequestMoreFeedbackTimeDays(0); - request.post("/api/courses", course, HttpStatus.BAD_REQUEST); + request.getMvc().perform(buildCreateCourse(course)).andExpect(status().isBadRequest()); List repoContent = courseRepo.findAll(); assertThat(repoContent).as("Course has not been stored").isEmpty(); // change configuration course.setMaxComplaintTimeDays(1); course.setMaxComplaints(0); - request.post("/api/courses", course, HttpStatus.BAD_REQUEST); + request.getMvc().perform(buildCreateCourse(course)).andExpect(status().isBadRequest()); repoContent = courseRepo.findAll(); assertThat(repoContent).as("Course has not been stored").isEmpty(); // change configuration again course.setMaxComplaintTimeDays(0); course.setMaxRequestMoreFeedbackTimeDays(-1); - request.post("/api/courses", course, HttpStatus.BAD_REQUEST); + request.getMvc().perform(buildCreateCourse(course)).andExpect(status().isBadRequest()); repoContent = courseRepo.findAll(); assertThat(repoContent).as("Course has not been stored").isEmpty(); } @@ -278,7 +295,8 @@ public void testCreateCourseWithCustomNonExistingGroupNames() throws Exception { course.setTeachingAssistantGroupName("TeachingAssistantGroupName"); course.setEditorGroupName("EditorGroupName"); course.setInstructorGroupName("InstructorGroupName"); - request.post("/api/courses", course, HttpStatus.CREATED); + + request.getMvc().perform(buildCreateCourse(course)).andExpect(status().isCreated()); List repoContent = courseRepo.findAll(); assertThat(repoContent).as("Course got stored").hasSize(1); } @@ -292,7 +310,8 @@ public void testCreateCourseWithOptions() throws Exception { mockDelegate.mockCreateGroupInUserManagement(course.getDefaultTeachingAssistantGroupName()); mockDelegate.mockCreateGroupInUserManagement(course.getDefaultEditorGroupName()); mockDelegate.mockCreateGroupInUserManagement(course.getDefaultInstructorGroupName()); - course = request.postWithResponseBody("/api/courses", course, Course.class, HttpStatus.CREATED); + MvcResult result = request.getMvc().perform(buildCreateCourse(course)).andExpect(status().isCreated()).andReturn(); + course = objectMapper.readValue(result.getResponse().getContentAsString(), Course.class); // Because the courseId is automatically generated we cannot use the findById method to retrieve the saved course. Course getFromRepo = courseRepo.findAll().get(0); assertThat(getFromRepo.getMaxComplaints()).as("Course has right maxComplaints Value").isEqualTo(5); @@ -306,7 +325,8 @@ public void testCreateCourseWithOptions() throws Exception { course.setMaxComplaintTimeDays(7); course.setPostsEnabled(true); course.setMaxRequestMoreFeedbackTimeDays(7); - Course updatedCourse = request.putWithResponseBody("/api/courses", course, Course.class, HttpStatus.OK); + result = request.getMvc().perform(buildUpdateCourse(getFromRepo.getId(), course)).andExpect(status().isOk()).andReturn(); + Course updatedCourse = objectMapper.readValue(result.getResponse().getContentAsString(), Course.class); assertThat(updatedCourse.getMaxComplaints()).as("maxComplaints Value updated successfully").isEqualTo(course.getMaxComplaints()); assertThat(updatedCourse.getMaxComplaintTimeDays()).as("maxComplaintTimeDays Value updated successfully").isEqualTo(course.getMaxComplaintTimeDays()); assertThat(updatedCourse.getPostsEnabled()).as("postsEnabled Value updated successfully").isTrue(); @@ -379,7 +399,7 @@ public void testDeleteNotExistingCourse() throws Exception { // Test public void testCreateCourseWithoutPermission() throws Exception { Course course = ModelFactory.generateCourse(null, null, null, new HashSet<>()); - request.post("/api/courses", course, HttpStatus.FORBIDDEN); + request.getMvc().perform(buildCreateCourse(course)).andExpect(status().isForbidden()); assertThat(courseRepo.findAll()).as("Course got stored").isEmpty(); } @@ -387,40 +407,13 @@ public void testCreateCourseWithoutPermission() throws Exception { public void testCreateCourseWithWrongShortName() throws Exception { Course course = ModelFactory.generateCourse(null, null, null, new HashSet<>()); course.setShortName("`badName~"); - request.post("/api/courses", course, HttpStatus.BAD_REQUEST); - } - - // Test - public void testUpdateCourseWithWrongShortName() throws Exception { - Course course = ModelFactory.generateCourse(null, null, null, new HashSet<>()); - course.setShortName("`badName~"); - courseRepo.save(course); - request.put("/api/courses", course, HttpStatus.BAD_REQUEST); - } - - // Test - public void testUpdateCourseWithoutId() throws Exception { - Course course = ModelFactory.generateCourse(null, null, null, new HashSet<>()); - - mockDelegate.mockCreateGroupInUserManagement(course.getDefaultStudentGroupName()); - mockDelegate.mockCreateGroupInUserManagement(course.getDefaultTeachingAssistantGroupName()); - mockDelegate.mockCreateGroupInUserManagement(course.getDefaultEditorGroupName()); - mockDelegate.mockCreateGroupInUserManagement(course.getDefaultInstructorGroupName()); - request.put("/api/courses", course, HttpStatus.CREATED); - List repoContent = courseRepo.findAll(); - assertThat(repoContent).as("Course got stored").hasSize(1); - } - - // Test - public void testUpdateCourseWithoutIdAsInstructor() throws Exception { - Course course = ModelFactory.generateCourse(null, null, null, new HashSet<>()); - request.put("/api/courses", course, HttpStatus.FORBIDDEN); + request.getMvc().perform(buildCreateCourse(course)).andExpect(status().isBadRequest()); } // Test public void testUpdateCourseIsEmpty() throws Exception { Course course = ModelFactory.generateCourse(1L, null, null, new HashSet<>()); - request.put("/api/courses", course, HttpStatus.NOT_FOUND); + request.getMvc().perform(buildCreateCourse(course)).andExpect(status().isBadRequest()); } // Test @@ -431,7 +424,8 @@ public void testEditCourseWithPermission() throws Exception { course.setTitle("Test Course"); course.setStartDate(ZonedDateTime.now().minusDays(5)); course.setEndDate(ZonedDateTime.now().plusDays(5)); - Course updatedCourse = request.putWithResponseBody("/api/courses", course, Course.class, HttpStatus.OK); + MvcResult result = request.getMvc().perform(buildUpdateCourse(course.getId(), course)).andExpect(status().isOk()).andReturn(); + Course updatedCourse = objectMapper.readValue(result.getResponse().getContentAsString(), Course.class); assertThat(updatedCourse.getShortName()).as("short name was changed correctly").isEqualTo(course.getShortName()); assertThat(updatedCourse.getTitle()).as("title was changed correctly").isEqualTo(course.getTitle()); assertThat(updatedCourse.getStartDate()).as("start date was changed correctly").isEqualTo(course.getStartDate()); @@ -455,7 +449,7 @@ public void testEditCourseShouldPreserveAssociations() throws Exception { course.setPrerequisites(prerequisites); course = courseRepo.save(course); - request.putWithResponseBody("/api/courses", course, Course.class, HttpStatus.OK); + request.getMvc().perform(buildUpdateCourse(course.getId(), course)).andExpect(status().isOk()); Course updatedCourse = courseRepo.findByIdWithOrganizationsAndLearningGoalsElseThrow(course.getId()); assertThat(updatedCourse.getOrganizations()).containsExactlyElementsOf(organizations); @@ -479,13 +473,14 @@ public void testUpdateCourseGroups() throws Exception { user.setGroups(Set.of("new-instructor-group")); userRepo.save(user); - // Create teaching assisstant in the course + // Create teaching assistant in the course user = ModelFactory.generateActivatedUser("teaching-assisstant11"); user.setGroups(Set.of("new-ta-group")); userRepo.save(user); mockDelegate.mockUpdateCoursePermissions(course, oldInstructorGroup, oldEditorGroup, oldTeachingAssistantGroup); - Course updatedCourse = request.putWithResponseBody("/api/courses", course, Course.class, HttpStatus.OK); + MvcResult result = request.getMvc().perform(buildUpdateCourse(course.getId(), course)).andExpect(status().isOk()).andReturn(); + Course updatedCourse = objectMapper.readValue(result.getResponse().getContentAsString(), Course.class); assertThat(updatedCourse.getInstructorGroupName()).isEqualTo("new-instructor-group"); assertThat(updatedCourse.getEditorGroupName()).isEqualTo("new-editor-group"); @@ -504,9 +499,7 @@ public void testUpdateCourseGroups_InExternalCiUserManagement_failToRemoveUser() course.setTeachingAssistantGroupName("new-ta-group"); mockDelegate.mockFailUpdateCoursePermissionsInCi(course, oldInstructorGroup, oldEditorGroup, oldTeachingAssistantGroup, false, true); - Course updatedCourse = request.putWithResponseBody("/api/courses", course, Course.class, HttpStatus.INTERNAL_SERVER_ERROR); - - assertThat(updatedCourse).isNull(); + request.getMvc().perform(buildUpdateCourse(course.getId(), course)).andExpect(status().isInternalServerError()).andReturn(); } // Test @@ -521,9 +514,7 @@ public void testUpdateCourseGroups_InExternalCiUserManagement_failToAddUser() th course.setTeachingAssistantGroupName("new-ta-group"); mockDelegate.mockFailUpdateCoursePermissionsInCi(course, oldInstructorGroup, oldEditorGroup, oldTeachingAssistantGroup, true, false); - Course updatedCourse = request.putWithResponseBody("/api/courses", course, Course.class, HttpStatus.INTERNAL_SERVER_ERROR); - - assertThat(updatedCourse).isNull(); + request.getMvc().perform(buildUpdateCourse(course.getId(), course)).andExpect(status().isInternalServerError()); } // Test @@ -1060,7 +1051,7 @@ public void testUpdateCourse_instructorNotInCourse() throws Exception { var course = ModelFactory.generateCourse(1L, null, null, new HashSet<>(), "tumuser", "tutor", "editor", "instructor"); course = courseRepo.save(course); - request.put("/api/courses", course, HttpStatus.FORBIDDEN); + request.getMvc().perform(buildUpdateCourse(course.getId(), course)).andExpect(status().isForbidden()); } // Test @@ -1081,7 +1072,8 @@ public void testGetAllStudentsOrTutorsOrInstructorsInCourse() throws Exception { assertThat(instructors).as("All instructors in course found").hasSize(numberOfInstructors); } - /** Searches for others users of a course in multiple roles + /** + * Searches for others users of a course in multiple roles * * @throws Exception */ @@ -1112,7 +1104,8 @@ public void testSearchStudentsAndTutorsAndInstructorsInCourse() throws Exception assertThat(instructor).as("An instructors in course found").hasSize(1); } - /** Tries to search for users of another course and expects to be forbidden + /** + * Tries to search for users of another course and expects to be forbidden * * @throws Exception */ @@ -2096,13 +2089,13 @@ public void testAddUsersToCourseGroup(String group, String registrationNumber1, // Test public void testCreateCourseWithValidStartAndEndDate() throws Exception { Course course = ModelFactory.generateCourse(null, ZonedDateTime.now().minusDays(1), ZonedDateTime.now(), new HashSet<>(), "student", "tutor", "editor", "instructor"); - request.post("/api/courses", course, HttpStatus.CREATED); + request.getMvc().perform(buildCreateCourse(course)).andExpect(status().isCreated()); } // Test public void testCreateCourseWithInvalidStartAndEndDate() throws Exception { Course course = ModelFactory.generateCourse(null, ZonedDateTime.now().plusDays(1), ZonedDateTime.now(), new HashSet<>(), "student", "tutor", "editor", "instructor"); - request.post("/api/courses", course, HttpStatus.BAD_REQUEST); + request.getMvc().perform(buildCreateCourse(course)).andExpect(status().isBadRequest()); } // Test @@ -2112,20 +2105,20 @@ public void testCreateInvalidOnlineCourse() throws Exception { // with RegistrationEnabled course.setOnlineCourse(true); course.setRegistrationEnabled(true); - request.post("/api/courses", course, HttpStatus.BAD_REQUEST); + request.getMvc().perform(buildCreateCourse(course)).andExpect(status().isBadRequest()); // without onlinecourseconfiguration course.setRegistrationEnabled(false); course.setOnlineCourseConfiguration(null); - request.post("/api/courses", course, HttpStatus.BAD_REQUEST); + request.getMvc().perform(buildCreateCourse(course)).andExpect(status().isBadRequest()); // with invalid online course configuration - no key and secret ModelFactory.generateOnlineCourseConfiguration(course, null, null, null, null); - request.post("/api/courses", course, HttpStatus.BAD_REQUEST); + request.getMvc().perform(buildCreateCourse(course)).andExpect(status().isBadRequest()); // with invalid user prefix - not matching regex ModelFactory.generateOnlineCourseConfiguration(course, "key", "secret", "with space", null); - request.post("/api/courses", course, HttpStatus.BAD_REQUEST); + request.getMvc().perform(buildCreateCourse(course)).andExpect(status().isBadRequest()); } // Test @@ -2133,8 +2126,9 @@ public void testCreateValidOnlineCourse() throws Exception { Course course = ModelFactory.generateCourse(null, ZonedDateTime.now().minusDays(1), ZonedDateTime.now(), new HashSet<>(), "student", "tutor", "editor", "instructor"); course.setOnlineCourse(true); ModelFactory.generateOnlineCourseConfiguration(course, "key", "secret", "validprefix", null); - course = request.postWithResponseBody("/api/courses", course, Course.class, HttpStatus.CREATED); - Course courseWithOnlineConfiguration = courseRepo.findByIdWithEagerOnlineCourseConfigurationElseThrow(course.getId()); + MvcResult result = request.getMvc().perform(buildCreateCourse(course)).andExpect(status().isCreated()).andReturn(); + Course createdCourse = objectMapper.readValue(result.getResponse().getContentAsString(), Course.class); + Course courseWithOnlineConfiguration = courseRepo.findByIdWithEagerOnlineCourseConfigurationElseThrow(createdCourse.getId()); assertThat(courseWithOnlineConfiguration.getOnlineCourseConfiguration()).isNotNull(); } @@ -2157,20 +2151,22 @@ public void testUpdateOnlineCourseConfiguration() throws Exception { course.setOnlineCourse(true); ModelFactory.generateOnlineCourseConfiguration(course, "key", "secret", "validprefix", null); - course = request.postWithResponseBody("/api/courses", course, Course.class, HttpStatus.CREATED); + MvcResult result = request.getMvc().perform(buildCreateCourse(course)).andExpect(status().isCreated()).andReturn(); + Course createdCourse = objectMapper.readValue(result.getResponse().getContentAsString(), Course.class); - OnlineCourseConfiguration onlineCourseConfiguration = course.getOnlineCourseConfiguration(); + OnlineCourseConfiguration onlineCourseConfiguration = createdCourse.getOnlineCourseConfiguration(); onlineCourseConfiguration.setLtiKey("changedKey"); onlineCourseConfiguration.setLtiSecret("changedSecret"); onlineCourseConfiguration.setUserPrefix("changedUserPrefix"); - course = request.putWithResponseBody("/api/courses", course, Course.class, HttpStatus.OK); + result = request.getMvc().perform(buildUpdateCourse(createdCourse.getId(), createdCourse)).andExpect(status().isOk()).andReturn(); + Course updatedCourse = objectMapper.readValue(result.getResponse().getContentAsString(), Course.class); - course = courseRepo.findByIdWithEagerOnlineCourseConfigurationElseThrow(course.getId()); + Course actualCourse = courseRepo.findByIdWithEagerOnlineCourseConfigurationElseThrow(updatedCourse.getId()); - assertThat(course.getOnlineCourseConfiguration().getLtiKey()).isEqualTo("changedKey"); - assertThat(course.getOnlineCourseConfiguration().getLtiSecret()).isEqualTo("changedSecret"); - assertThat(course.getOnlineCourseConfiguration().getUserPrefix()).isEqualTo("changedUserPrefix"); + assertThat(actualCourse.getOnlineCourseConfiguration().getLtiKey()).isEqualTo("changedKey"); + assertThat(actualCourse.getOnlineCourseConfiguration().getLtiSecret()).isEqualTo("changedSecret"); + assertThat(actualCourse.getOnlineCourseConfiguration().getUserPrefix()).isEqualTo("changedUserPrefix"); } // Test @@ -2179,13 +2175,15 @@ public void testUpdateCourseRemoveOnlineCourseConfiguration() throws Exception { course.setOnlineCourse(true); ModelFactory.generateOnlineCourseConfiguration(course, "key", "secret", "validprefix", null); - course = request.postWithResponseBody("/api/courses", course, Course.class, HttpStatus.CREATED); + MvcResult result = request.getMvc().perform(buildCreateCourse(course)).andExpect(status().isCreated()).andReturn(); + Course createdCourse = objectMapper.readValue(result.getResponse().getContentAsString(), Course.class); - course.setOnlineCourse(false); - course.setOnlineCourseConfiguration(null); - course = request.putWithResponseBody("/api/courses", course, Course.class, HttpStatus.OK); + createdCourse.setOnlineCourse(false); + createdCourse.setOnlineCourseConfiguration(null); + result = request.getMvc().perform(buildUpdateCourse(createdCourse.getId(), createdCourse)).andExpect(status().isOk()).andReturn(); + Course updatedCourse = objectMapper.readValue(result.getResponse().getContentAsString(), Course.class); - Course courseWithoutOnlineConfiguration = courseRepo.findByIdWithEagerOnlineCourseConfigurationElseThrow(course.getId()); + Course courseWithoutOnlineConfiguration = courseRepo.findByIdWithEagerOnlineCourseConfigurationElseThrow(updatedCourse.getId()); assertThat(courseWithoutOnlineConfiguration.getOnlineCourseConfiguration()).isNull(); } @@ -2200,4 +2198,32 @@ public void testDeleteCourseDeletesOnlineConfiguration() throws Exception { assertThat(onlineCourseConfigurationRepository.findById(course.getOnlineCourseConfiguration().getId())).isNotPresent(); } + + public MockHttpServletRequestBuilder buildCreateCourse(@NotNull Course course) throws JsonProcessingException { + return buildCreateCourse(course, null); + } + + public MockHttpServletRequestBuilder buildCreateCourse(@NotNull Course course, String fileContent) throws JsonProcessingException { + var coursePart = new MockMultipartFile("course", "", MediaType.APPLICATION_JSON_VALUE, objectMapper.writeValueAsString(course).getBytes()); + var builder = MockMvcRequestBuilders.multipart(HttpMethod.POST, "/api/courses").file(coursePart); + if (fileContent != null) { + var filePart = new MockMultipartFile("file", "test.zip", MediaType.APPLICATION_OCTET_STREAM_VALUE, fileContent.getBytes()); + builder.file(filePart); + } + return builder.contentType(MediaType.MULTIPART_FORM_DATA_VALUE); + } + + public MockHttpServletRequestBuilder buildUpdateCourse(long id, @NotNull Course course) throws JsonProcessingException { + return buildUpdateCourse(id, course, null); + } + + public MockHttpServletRequestBuilder buildUpdateCourse(long id, @NotNull Course course, String fileContent) throws JsonProcessingException { + var coursePart = new MockMultipartFile("course", "", MediaType.APPLICATION_JSON_VALUE, objectMapper.writeValueAsString(course).getBytes()); + var builder = MockMvcRequestBuilders.multipart(HttpMethod.PUT, "/api/courses/" + id).file(coursePart); + if (fileContent != null) { + var filePart = new MockMultipartFile("file", "test.zip", MediaType.APPLICATION_OCTET_STREAM_VALUE, fileContent.getBytes()); + builder.file(filePart); + } + return builder.contentType(MediaType.MULTIPART_FORM_DATA_VALUE); + } } From a3f36fb8ae7cfe358cb764b0873bfea8aa6f6680 Mon Sep 17 00:00:00 2001 From: Julian Christl Date: Thu, 13 Oct 2022 17:37:08 +0200 Subject: [PATCH 06/16] fix client test --- .../spec/component/course/course-update.component.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/javascript/spec/component/course/course-update.component.spec.ts b/src/test/javascript/spec/component/course/course-update.component.spec.ts index 056927914b88..51afa9fb432e 100644 --- a/src/test/javascript/spec/component/course/course-update.component.spec.ts +++ b/src/test/javascript/spec/component/course/course-update.component.spec.ts @@ -250,7 +250,7 @@ describe('Course Management Update Component', () => { length: 1, item: () => file, } as unknown as FileList; - const event = { target: { files: fileList } } as unknown as Event; + const event = { currentTarget: { files: fileList } } as unknown as Event; comp.setCourseImage(event); expect(comp.courseImageUploadFile).toEqual(file); }); From 117ea9e78a9b5a165cef274bca4acd73c2e7e744 Mon Sep 17 00:00:00 2001 From: Julian Christl Date: Tue, 18 Oct 2022 19:59:50 +0200 Subject: [PATCH 07/16] fix cypress tests --- .../cypress/integration/CourseManagement.spec.ts | 11 ++++++----- .../integration/exam/ExamAssessment.spec.ts | 4 ++-- .../exam/ExamCreationDeletion.spec.ts | 4 ++-- .../exam/ExamDateVerification.spec.ts | 4 ++-- .../integration/exam/ExamManagement.spec.ts | 4 ++-- .../integration/exam/ExamParticipation.spec.ts | 4 ++-- .../modeling/ModelingExerciseAssessment.spec.ts | 3 ++- .../modeling/ModelingExerciseManagement.spec.ts | 5 +++-- .../ModelingExerciseParticipation.spec.ts | 5 +++-- .../ProgrammingExerciseAssessment.spec.ts | 4 ++-- .../ProgrammingExerciseManagement.spec.ts | 13 +++++-------- .../ProgrammingExerciseParticipation.spec.ts | 3 ++- ...ProgrammingExerciseStaticCodeAnalysis.spec.ts | 3 ++- .../quiz-exercise/QuizExerciseAssessment.spec.ts | 3 ++- .../quiz-exercise/QuizExerciseManagement.spec.ts | 3 ++- .../QuizExerciseParticipation.spec.ts | 3 ++- .../text/TextExerciseAssessment.spec.ts | 3 ++- .../text/TextExerciseManagement.spec.ts | 3 ++- .../text/TextExerciseParticipation.spec.ts | 3 ++- .../lecture/LectureManagement.spec.ts | 3 ++- .../support/requests/CourseManagementRequests.ts | 16 ++++++++++++++-- src/test/cypress/support/utils.ts | 7 ++++++- 22 files changed, 69 insertions(+), 42 deletions(-) diff --git a/src/test/cypress/integration/CourseManagement.spec.ts b/src/test/cypress/integration/CourseManagement.spec.ts index db5f9269b7c7..85be00a8a8b2 100644 --- a/src/test/cypress/integration/CourseManagement.spec.ts +++ b/src/test/cypress/integration/CourseManagement.spec.ts @@ -1,12 +1,13 @@ import { Interception } from 'cypress/types/net-stubbing'; -import { COURSE_BASE } from '../support/requests/CourseManagementRequests'; +import { COURSE_BASE, parseCourseAfterMultiPart } from '../support/requests/CourseManagementRequests'; import { GET, BASE_API, POST } from '../support/constants'; import { artemis } from '../support/ArtemisTesting'; import { CourseManagementPage } from '../support/pageobjects/course/CourseManagementPage'; import { NavigationBar } from '../support/pageobjects/NavigationBar'; import { ArtemisRequests } from '../support/requests/ArtemisRequests'; -import { generateUUID } from '../support/utils'; +import { generateUUID, parseArrayBufferAsJsonObject } from '../support/utils'; import { Course } from 'app/entities/course.model'; +import * as Cypress from 'cypress'; // Requests const artemisRequests: ArtemisRequests = new ArtemisRequests(); @@ -40,9 +41,9 @@ describe('Course management', () => { let courseId: number; beforeEach(() => { - artemisRequests.courseManagement.createCourse(false, courseName, courseShortName).then((response: Cypress.Response) => { - course = response.body; - courseId = response.body!.id!; + artemisRequests.courseManagement.createCourse(false, courseName, courseShortName).then((response) => { + course = parseCourseAfterMultiPart(response); + courseId = course.id!; }); }); diff --git a/src/test/cypress/integration/exam/ExamAssessment.spec.ts b/src/test/cypress/integration/exam/ExamAssessment.spec.ts index 53a21228e951..3e5027b42c67 100644 --- a/src/test/cypress/integration/exam/ExamAssessment.spec.ts +++ b/src/test/cypress/integration/exam/ExamAssessment.spec.ts @@ -3,7 +3,7 @@ import { Course } from 'app/entities/course.model'; import { ExerciseGroup } from 'app/entities/exercise-group.model'; import { Exam } from 'app/entities/exam.model'; import { artemis } from '../../support/ArtemisTesting'; -import { CypressAssessmentType, CypressExamBuilder } from '../../support/requests/CourseManagementRequests'; +import { CypressAssessmentType, CypressExamBuilder, parseCourseAfterMultiPart } from '../../support/requests/CourseManagementRequests'; import partiallySuccessful from '../../fixtures/programming_exercise_submissions/partially_successful/submission.json'; import dayjs, { Dayjs } from 'dayjs/esm'; import textSubmission from '../../fixtures/text_exercise_submission/text_exercise_submission.json'; @@ -45,7 +45,7 @@ describe('Exam assessment', () => { before('Create a course', () => { cy.login(admin); courseManagementRequests.createCourse(true).then((response) => { - course = response.body; + course = parseCourseAfterMultiPart(response); courseManagementRequests.addStudentToCourse(course, artemis.users.getStudentOne()); courseManagementRequests.addTutorToCourse(course, artemis.users.getTutor()); }); diff --git a/src/test/cypress/integration/exam/ExamCreationDeletion.spec.ts b/src/test/cypress/integration/exam/ExamCreationDeletion.spec.ts index 8c6b564ec6d8..2dd5e7bb7c60 100644 --- a/src/test/cypress/integration/exam/ExamCreationDeletion.spec.ts +++ b/src/test/cypress/integration/exam/ExamCreationDeletion.spec.ts @@ -1,6 +1,6 @@ import { Interception } from 'cypress/types/net-stubbing'; import { Course } from 'app/entities/course.model'; -import { CypressExamBuilder } from '../../support/requests/CourseManagementRequests'; +import { CypressExamBuilder, parseCourseAfterMultiPart } from '../../support/requests/CourseManagementRequests'; import dayjs from 'dayjs/esm'; import { artemis } from '../../support/ArtemisTesting'; import { generateUUID } from '../../support/utils'; @@ -23,7 +23,7 @@ describe('Exam creation/deletion', () => { before(() => { cy.login(artemis.users.getAdmin()); courseManagementRequests.createCourse().then((response) => { - course = response.body; + course = parseCourseAfterMultiPart(response); }); }); diff --git a/src/test/cypress/integration/exam/ExamDateVerification.spec.ts b/src/test/cypress/integration/exam/ExamDateVerification.spec.ts index 301db125fb31..4efaad39f087 100644 --- a/src/test/cypress/integration/exam/ExamDateVerification.spec.ts +++ b/src/test/cypress/integration/exam/ExamDateVerification.spec.ts @@ -1,7 +1,7 @@ import { ExerciseGroup } from 'app/entities/exercise-group.model'; import { Course } from 'app/entities/course.model'; import { Exam } from 'app/entities/exam.model'; -import { CypressExamBuilder } from '../../support/requests/CourseManagementRequests'; +import { CypressExamBuilder, parseCourseAfterMultiPart } from '../../support/requests/CourseManagementRequests'; import dayjs from 'dayjs/esm'; import { artemis } from '../../support/ArtemisTesting'; import { generateUUID } from '../../support/utils'; @@ -22,7 +22,7 @@ describe('Exam date verification', () => { before(() => { cy.login(artemis.users.getAdmin()); courseManagementRequests.createCourse().then((response) => { - course = response.body; + course = parseCourseAfterMultiPart(response); courseManagementRequests.addStudentToCourse(course, artemis.users.getStudentOne()); }); }); diff --git a/src/test/cypress/integration/exam/ExamManagement.spec.ts b/src/test/cypress/integration/exam/ExamManagement.spec.ts index 91b299cc309a..b4440cdf8813 100644 --- a/src/test/cypress/integration/exam/ExamManagement.spec.ts +++ b/src/test/cypress/integration/exam/ExamManagement.spec.ts @@ -1,7 +1,7 @@ import { Interception } from 'cypress/types/net-stubbing'; import { Exam } from 'app/entities/exam.model'; import { Course } from 'app/entities/course.model'; -import { CypressExamBuilder } from '../../support/requests/CourseManagementRequests'; +import { CypressExamBuilder, parseCourseAfterMultiPart } from '../../support/requests/CourseManagementRequests'; import { artemis } from '../../support/ArtemisTesting'; import { generateUUID } from '../../support/utils'; @@ -31,7 +31,7 @@ describe('Exam management', () => { before(() => { cy.login(users.getAdmin()); courseManagementRequests.createCourse().then((response) => { - course = response.body; + course = parseCourseAfterMultiPart(response); courseManagementRequests.addStudentToCourse(course, users.getStudentOne()); const examConfig = new CypressExamBuilder(course).title(examTitle).build(); courseManagementRequests.createExam(examConfig).then((examResponse) => { diff --git a/src/test/cypress/integration/exam/ExamParticipation.spec.ts b/src/test/cypress/integration/exam/ExamParticipation.spec.ts index 4980201b019b..42a61d91676b 100644 --- a/src/test/cypress/integration/exam/ExamParticipation.spec.ts +++ b/src/test/cypress/integration/exam/ExamParticipation.spec.ts @@ -1,7 +1,7 @@ import { QuizExercise } from 'app/entities/quiz/quiz-exercise.model'; import { Exam } from 'app/entities/exam.model'; import { GET, BASE_API } from '../../support/constants'; -import { CypressExamBuilder } from '../../support/requests/CourseManagementRequests'; +import { CypressExamBuilder, parseCourseAfterMultiPart } from '../../support/requests/CourseManagementRequests'; import { artemis } from '../../support/ArtemisTesting'; import dayjs from 'dayjs/esm'; import submission from '../../fixtures/programming_exercise_submissions/all_successful/submission.json'; @@ -37,7 +37,7 @@ describe('Exam participation', () => { before(() => { cy.login(users.getAdmin()); courseRequests.createCourse(true).then((response) => { - course = response.body; + course = parseCourseAfterMultiPart(response); const examContent = new CypressExamBuilder(course) .visibleDate(dayjs().subtract(3, 'days')) .startDate(dayjs().subtract(2, 'days')) diff --git a/src/test/cypress/integration/exercises/modeling/ModelingExerciseAssessment.spec.ts b/src/test/cypress/integration/exercises/modeling/ModelingExerciseAssessment.spec.ts index 4d7ab35b9196..f13f814c491e 100644 --- a/src/test/cypress/integration/exercises/modeling/ModelingExerciseAssessment.spec.ts +++ b/src/test/cypress/integration/exercises/modeling/ModelingExerciseAssessment.spec.ts @@ -2,6 +2,7 @@ import { ModelingExercise } from 'app/entities/modeling-exercise.model'; import { Course } from 'app/entities/course.model'; import { artemis } from '../../../support/ArtemisTesting'; import day from 'dayjs/esm'; +import { parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; // pageobjects const assessmentEditor = artemis.pageobjects.assessment.modeling; @@ -85,7 +86,7 @@ describe('Modeling Exercise Assessment Spec', () => { function createCourseWithModelingExercise() { cy.login(admin); return courseManagementRequests.createCourse(true).then((response) => { - course = response.body; + course = parseCourseAfterMultiPart(response); courseManagementRequests.addStudentToCourse(course, student); courseManagementRequests.addTutorToCourse(course, tutor); courseManagementRequests.addInstructorToCourse(course, instructor); diff --git a/src/test/cypress/integration/exercises/modeling/ModelingExerciseManagement.spec.ts b/src/test/cypress/integration/exercises/modeling/ModelingExerciseManagement.spec.ts index 158feb904221..ce45d7683059 100644 --- a/src/test/cypress/integration/exercises/modeling/ModelingExerciseManagement.spec.ts +++ b/src/test/cypress/integration/exercises/modeling/ModelingExerciseManagement.spec.ts @@ -6,6 +6,7 @@ import { MODELING_EDITOR_CANVAS } from '../../../support/pageobjects/exercises/m // https://day.js.org/docs is a tool for date/time import dayjs from 'dayjs/esm'; +import { parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; // pageobjects const createModelingExercise = artemis.pageobjects.exercise.modeling.creation; @@ -27,8 +28,8 @@ const modelingExerciseTitle = 'Cypress modeling exercise'; describe('Modeling Exercise Management Spec', () => { before('Create a course', () => { cy.login(admin); - courseManagementRequests.createCourse().then((courseResp: Cypress.Response) => { - course = courseResp.body; + courseManagementRequests.createCourse().then((response: Cypress.Response) => { + course = parseCourseAfterMultiPart(response); courseManagementRequests.addInstructorToCourse(course, instructor); courseManagementRequests.addStudentToCourse(course, student); }); diff --git a/src/test/cypress/integration/exercises/modeling/ModelingExerciseParticipation.spec.ts b/src/test/cypress/integration/exercises/modeling/ModelingExerciseParticipation.spec.ts index 560975eab40d..720a41b17ad5 100644 --- a/src/test/cypress/integration/exercises/modeling/ModelingExerciseParticipation.spec.ts +++ b/src/test/cypress/integration/exercises/modeling/ModelingExerciseParticipation.spec.ts @@ -1,6 +1,7 @@ import { ModelingExercise } from 'app/entities/modeling-exercise.model'; import { Course } from 'app/entities/course.model'; import { artemis } from '../../../support/ArtemisTesting'; +import { parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; // pageobjects const courseManagement = artemis.pageobjects.course.management; @@ -18,8 +19,8 @@ let modelingExercise: ModelingExercise; describe('Modeling Exercise Participation Spec', () => { before('Log in as admin and create a course', () => { cy.login(admin); - courseManagementRequests.createCourse().then((courseResp: Cypress.Response) => { - course = courseResp.body; + courseManagementRequests.createCourse().then((response: Cypress.Response) => { + course = parseCourseAfterMultiPart(response); cy.visit(`/course-management/${course.id}`); courseManagement.addStudentToCourse(student); courseManagementRequests.createModelingExercise({ course }).then((resp: Cypress.Response) => { diff --git a/src/test/cypress/integration/exercises/programming/ProgrammingExerciseAssessment.spec.ts b/src/test/cypress/integration/exercises/programming/ProgrammingExerciseAssessment.spec.ts index 44e05830f437..c00a2261ee34 100644 --- a/src/test/cypress/integration/exercises/programming/ProgrammingExerciseAssessment.spec.ts +++ b/src/test/cypress/integration/exercises/programming/ProgrammingExerciseAssessment.spec.ts @@ -1,7 +1,7 @@ import { Interception } from 'cypress/types/net-stubbing'; import { ProgrammingExercise } from 'app/entities/programming-exercise.model'; import { Course } from 'app/entities/course.model'; -import { CypressAssessmentType } from '../../../support/requests/CourseManagementRequests'; +import { CypressAssessmentType, parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; import { artemis } from 'src/test/cypress/support/ArtemisTesting'; import dayjs from 'dayjs/esm'; @@ -96,7 +96,7 @@ describe('Programming exercise assessment', () => { function createCourseWithProgrammingExercise() { cy.login(admin); return courseManagement.createCourse(true).then((response) => { - course = response.body; + course = parseCourseAfterMultiPart(response); courseManagement.addStudentToCourse(course, student); courseManagement.addTutorToCourse(course, tutor); courseManagement.addInstructorToCourse(course, instructor); diff --git a/src/test/cypress/integration/exercises/programming/ProgrammingExerciseManagement.spec.ts b/src/test/cypress/integration/exercises/programming/ProgrammingExerciseManagement.spec.ts index 363413f9f589..cd9dffa60326 100644 --- a/src/test/cypress/integration/exercises/programming/ProgrammingExerciseManagement.spec.ts +++ b/src/test/cypress/integration/exercises/programming/ProgrammingExerciseManagement.spec.ts @@ -4,7 +4,7 @@ import { Course } from 'app/entities/course.model'; import { DELETE } from '../../../support/constants'; import { artemis } from '../../../support/ArtemisTesting'; import { generateUUID } from '../../../support/utils'; -import { PROGRAMMING_EXERCISE_BASE } from '../../../support/requests/CourseManagementRequests'; +import { parseCourseAfterMultiPart, PROGRAMMING_EXERCISE_BASE } from '../../../support/requests/CourseManagementRequests'; // Admin account const admin = artemis.users.getAdmin(); @@ -23,13 +23,10 @@ describe('Programming Exercise Management', () => { before(() => { cy.login(admin); - artemisRequests.courseManagement - .createCourse(true) - .its('body') - .then((body) => { - expect(body).property('id').to.be.a('number'); - course = body; - }); + artemisRequests.courseManagement.createCourse(true).then((response) => { + course = parseCourseAfterMultiPart(response); + expect(course).property('id').to.be.a('number'); + }); }); describe('Programming exercise deletion', () => { diff --git a/src/test/cypress/integration/exercises/programming/ProgrammingExerciseParticipation.spec.ts b/src/test/cypress/integration/exercises/programming/ProgrammingExerciseParticipation.spec.ts index e5e7d2a2e91d..952f7bbe4bec 100644 --- a/src/test/cypress/integration/exercises/programming/ProgrammingExerciseParticipation.spec.ts +++ b/src/test/cypress/integration/exercises/programming/ProgrammingExerciseParticipation.spec.ts @@ -4,6 +4,7 @@ import allSuccessful from '../../../fixtures/programming_exercise_submissions/al import partiallySuccessful from '../../../fixtures/programming_exercise_submissions/partially_successful/submission.json'; import { artemis } from '../../../support/ArtemisTesting'; import { makeSubmissionAndVerifyResults, startParticipationInProgrammingExercise } from '../../../support/pageobjects/exercises/programming/OnlineEditorPage'; +import { parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; // The user management object const users = artemis.users; @@ -50,7 +51,7 @@ describe('Programming exercise participations', () => { function setupCourseAndProgrammingExercise() { cy.login(users.getAdmin(), '/'); courseManagement.createCourse(true).then((response) => { - course = response.body; + course = parseCourseAfterMultiPart(response); courseManagement.addStudentToCourse(course, users.getStudentOne()); courseManagement.addStudentToCourse(course, users.getStudentTwo()); courseManagement.addStudentToCourse(course, users.getStudentThree()); diff --git a/src/test/cypress/integration/exercises/programming/ProgrammingExerciseStaticCodeAnalysis.spec.ts b/src/test/cypress/integration/exercises/programming/ProgrammingExerciseStaticCodeAnalysis.spec.ts index 8f4f3610ba95..69c6dabff5d1 100644 --- a/src/test/cypress/integration/exercises/programming/ProgrammingExerciseStaticCodeAnalysis.spec.ts +++ b/src/test/cypress/integration/exercises/programming/ProgrammingExerciseStaticCodeAnalysis.spec.ts @@ -3,6 +3,7 @@ import { Course } from 'app/entities/course.model'; import scaSubmission from '../../../fixtures/programming_exercise_submissions/static_code_analysis/submission.json'; import { artemis } from '../../../support/ArtemisTesting'; import { makeSubmissionAndVerifyResults, startParticipationInProgrammingExercise } from '../../../support/pageobjects/exercises/programming/OnlineEditorPage'; +import { parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; // The user management object const users = artemis.users; @@ -42,7 +43,7 @@ describe('Static code analysis tests', () => { function setupCourseAndProgrammingExercise() { cy.login(users.getAdmin()); courseManagement.createCourse(true).then((response) => { - course = response.body; + course = parseCourseAfterMultiPart(response); courseManagement.addStudentToCourse(course, users.getStudentOne()); courseManagement.createProgrammingExercise({ course }, 50).then((dto) => { exercise = dto.body; diff --git a/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseAssessment.spec.ts b/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseAssessment.spec.ts index d2db61dfbbf7..c1ed1586ca69 100644 --- a/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseAssessment.spec.ts +++ b/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseAssessment.spec.ts @@ -3,6 +3,7 @@ import { Course } from 'app/entities/course.model'; import { artemis } from '../../../support/ArtemisTesting'; import shortAnswerQuizTemplate from '../../../fixtures/quiz_exercise_fixtures/shortAnswerQuiz_template.json'; import multipleChoiceQuizTemplate from '../../../fixtures/quiz_exercise_fixtures/multipleChoiceQuiz_template.json'; +import { parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; // Accounts const admin = artemis.users.getAdmin(); @@ -22,7 +23,7 @@ describe('Quiz Exercise Assessment', () => { before('Set up course', () => { cy.login(admin); courseManagementRequest.createCourse().then((response) => { - course = response.body; + course = parseCourseAfterMultiPart(response); courseManagementRequest.addStudentToCourse(course, student); courseManagementRequest.addTutorToCourse(course, tutor); }); diff --git a/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseManagement.spec.ts b/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseManagement.spec.ts index 6ac4e9790671..04e65d5a5572 100644 --- a/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseManagement.spec.ts +++ b/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseManagement.spec.ts @@ -5,6 +5,7 @@ import { artemis } from '../../../support/ArtemisTesting'; import { generateUUID } from '../../../support/utils'; import multipleChoiceTemplate from '../../../fixtures/quiz_exercise_fixtures/multipleChoiceQuiz_template.json'; import { DELETE } from '../../../support/constants'; +import { parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; // Accounts const admin = artemis.users.getAdmin(); @@ -24,7 +25,7 @@ describe('Quiz Exercise Management', () => { before('Set up course', () => { cy.login(admin); courseManagementRequest.createCourse().then((response) => { - course = response.body; + course = parseCourseAfterMultiPart(response); }); }); diff --git a/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseParticipation.spec.ts b/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseParticipation.spec.ts index b237349fa33a..f7193aadcb2a 100644 --- a/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseParticipation.spec.ts +++ b/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseParticipation.spec.ts @@ -3,6 +3,7 @@ import { Course } from 'app/entities/course.model'; import { artemis } from '../../../support/ArtemisTesting'; import multipleChoiceQuizTemplate from '../../../fixtures/quiz_exercise_fixtures/multipleChoiceQuiz_template.json'; import shortAnswerQuizTemplate from '../../../fixtures/quiz_exercise_fixtures/shortAnswerQuiz_template.json'; +import { parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; // Accounts const admin = artemis.users.getAdmin(); @@ -26,7 +27,7 @@ describe('Quiz Exercise Participation', () => { before('Set up course', () => { cy.login(admin); courseManagementRequest.createCourse().then((response) => { - course = response.body; + course = parseCourseAfterMultiPart(response); courseManagementRequest.addStudentToCourse(course, student); }); }); diff --git a/src/test/cypress/integration/exercises/text/TextExerciseAssessment.spec.ts b/src/test/cypress/integration/exercises/text/TextExerciseAssessment.spec.ts index 3d6f7d302000..dd38f1c0ba41 100644 --- a/src/test/cypress/integration/exercises/text/TextExerciseAssessment.spec.ts +++ b/src/test/cypress/integration/exercises/text/TextExerciseAssessment.spec.ts @@ -2,6 +2,7 @@ import { Interception } from 'cypress/types/net-stubbing'; import { TextExercise } from 'app/entities/text-exercise.model'; import { Course } from 'app/entities/course.model'; import { artemis } from 'src/test/cypress/support/ArtemisTesting'; +import { parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; // The user management object const users = artemis.users; @@ -87,7 +88,7 @@ describe('Text exercise assessment', () => { function createCourseWithTextExercise() { cy.login(admin); return courseManagement.createCourse().then((response) => { - course = response.body; + course = parseCourseAfterMultiPart(response); courseManagement.addStudentToCourse(course, student); courseManagement.addTutorToCourse(course, tutor); courseManagement.addInstructorToCourse(course, instructor); diff --git a/src/test/cypress/integration/exercises/text/TextExerciseManagement.spec.ts b/src/test/cypress/integration/exercises/text/TextExerciseManagement.spec.ts index 87fb5c7a7559..195975bc014b 100644 --- a/src/test/cypress/integration/exercises/text/TextExerciseManagement.spec.ts +++ b/src/test/cypress/integration/exercises/text/TextExerciseManagement.spec.ts @@ -6,6 +6,7 @@ import { DELETE } from '../../../support/constants'; import { generateUUID } from '../../../support/utils'; import { artemis } from '../../../support/ArtemisTesting'; import dayjs from 'dayjs/esm'; +import { parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; // The user management object const users = artemis.users; @@ -27,7 +28,7 @@ describe('Text exercise management', () => { before(() => { cy.login(users.getAdmin()); courseManagement.createCourse().then((response) => { - course = response.body; + course = parseCourseAfterMultiPart(response); }); }); diff --git a/src/test/cypress/integration/exercises/text/TextExerciseParticipation.spec.ts b/src/test/cypress/integration/exercises/text/TextExerciseParticipation.spec.ts index e571b29aa814..5cdf79db9f00 100644 --- a/src/test/cypress/integration/exercises/text/TextExerciseParticipation.spec.ts +++ b/src/test/cypress/integration/exercises/text/TextExerciseParticipation.spec.ts @@ -2,6 +2,7 @@ import { Interception } from 'cypress/types/net-stubbing'; import { TextExercise } from 'app/entities/text-exercise.model'; import { Course } from 'app/entities/course.model'; import { artemis } from '../../../support/ArtemisTesting'; +import { parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; // The user management object const users = artemis.users; @@ -20,7 +21,7 @@ describe('Text exercise participation', () => { before(() => { cy.login(users.getAdmin()); courseManagement.createCourse().then((response) => { - course = response.body; + course = parseCourseAfterMultiPart(response); courseManagement.addStudentToCourse(course, users.getStudentOne()); courseManagement.createTextExercise({ course }).then((exerciseResponse: Cypress.Response) => { exercise = exerciseResponse.body; diff --git a/src/test/cypress/integration/lecture/LectureManagement.spec.ts b/src/test/cypress/integration/lecture/LectureManagement.spec.ts index 46903dffac98..7d544c62f665 100644 --- a/src/test/cypress/integration/lecture/LectureManagement.spec.ts +++ b/src/test/cypress/integration/lecture/LectureManagement.spec.ts @@ -3,6 +3,7 @@ import { Course } from 'app/entities/course.model'; import { artemis } from '../../support/ArtemisTesting'; import { generateUUID } from '../../support/utils'; import dayjs from 'dayjs/esm'; +import { parseCourseAfterMultiPart } from '../../support/requests/CourseManagementRequests'; // Requests const courseManagementRequests = artemis.requests.courseManagement; @@ -22,7 +23,7 @@ describe('Lecture management', () => { before(() => { cy.login(admin); courseManagementRequests.createCourse().then((response) => { - course = response.body; + course = parseCourseAfterMultiPart(response); courseManagementRequests.addInstructorToCourse(course, instructor); }); }); diff --git a/src/test/cypress/support/requests/CourseManagementRequests.ts b/src/test/cypress/support/requests/CourseManagementRequests.ts index e11dd1b6ab74..6fa06636222d 100644 --- a/src/test/cypress/support/requests/CourseManagementRequests.ts +++ b/src/test/cypress/support/requests/CourseManagementRequests.ts @@ -7,7 +7,7 @@ import { ProgrammingExercise } from 'app/entities/programming-exercise.model'; import { Course } from 'app/entities/course.model'; import { BASE_API, DELETE, POST, PUT, GET } from '../constants'; import programmingExerciseTemplate from '../../fixtures/requests/programming_exercise_template.json'; -import { dayjsToString, generateUUID } from '../utils'; +import { dayjsToString, generateUUID, parseArrayBufferAsJsonObject } from '../utils'; import examTemplate from '../../fixtures/requests/exam_template.json'; import day from 'dayjs/esm'; import { CypressCredentials } from '../users'; @@ -74,10 +74,13 @@ export class CourseManagementRequests { course.editorGroupName = Cypress.env('editorGroupName'); course.instructorGroupName = Cypress.env('instructorGroupName'); } + const formData = new FormData(); + Cypress.Blob; + formData.append('course', new File([JSON.stringify(course)], 'course', { type: 'application/json' })); return cy.request({ url: BASE_API + 'courses', method: POST, - body: course, + body: formData, }); } @@ -630,3 +633,12 @@ export enum CypressExerciseType { TEXT, QUIZ, } + +export function parseCourseAfterMultiPart(response: Cypress.Response): Course { + // Cypress currently has some issues with our multipart request, parsing this not as an object but as an ArrayBuffer + // Once this is fixed (and hence the expect statements below fail), we can remove the additional parsing + expect(response.body).not.to.be.an('object'); + expect(response.body).to.be.an('ArrayBuffer'); + + return parseArrayBufferAsJsonObject(response.body as ArrayBuffer); +} diff --git a/src/test/cypress/support/utils.ts b/src/test/cypress/support/utils.ts index 21bc3eadfc5d..885a75590888 100644 --- a/src/test/cypress/support/utils.ts +++ b/src/test/cypress/support/utils.ts @@ -23,6 +23,11 @@ export function generateUUID() { * @returns a formatted string representing the date with utc timezone */ export function dayjsToString(dayjs: day.Dayjs) { - // We need to add the Z at the end. Otherwise the server can't parse it. + // We need to add the Z at the end. Otherwise, the server can't parse it. return dayjs.utc().format(TIME_FORMAT) + 'Z'; } + +export function parseArrayBufferAsJsonObject(buffer: ArrayBuffer) { + const bodyString = Cypress.Blob.arrayBufferToBinaryString(buffer); + return JSON.parse(bodyString); +} From 00300725666c9679b97285b7baa2318cbea933f2 Mon Sep 17 00:00:00 2001 From: Julian Christl Date: Thu, 20 Oct 2022 14:39:01 +0200 Subject: [PATCH 08/16] add server tests for file upload --- ...rseBitbucketBambooJiraIntegrationTest.java | 24 ++++++ .../CourseGitlabJenkinsIntegrationTest.java | 24 ++++++ .../www1/artemis/util/CourseTestService.java | 83 ++++++++++++++++++- 3 files changed, 129 insertions(+), 2 deletions(-) diff --git a/src/test/java/de/tum/in/www1/artemis/CourseBitbucketBambooJiraIntegrationTest.java b/src/test/java/de/tum/in/www1/artemis/CourseBitbucketBambooJiraIntegrationTest.java index bcbeb65ddf66..a716f853ae89 100644 --- a/src/test/java/de/tum/in/www1/artemis/CourseBitbucketBambooJiraIntegrationTest.java +++ b/src/test/java/de/tum/in/www1/artemis/CourseBitbucketBambooJiraIntegrationTest.java @@ -161,6 +161,30 @@ void testUpdateCourseGroups() throws Exception { courseTestService.testUpdateCourseGroups(); } + @Test + @WithMockUser(username = "admin", roles = "ADMIN") + public void testCreateAndUpdateCourseWithCourseImage() throws Exception { + courseTestService.testCreateAndUpdateCourseWithCourseImage(); + } + + @Test + @WithMockUser(username = "admin", roles = "ADMIN") + public void testCreateAndUpdateCourseWithPersistentCourseImageOnUpdate() throws Exception { + courseTestService.testCreateAndUpdateCourseWithPersistentCourseImageOnUpdate(); + } + + @Test + @WithMockUser(username = "admin", roles = "ADMIN") + public void testCreateAndUpdateCourseWithRemoveCourseImageOnUpdate() throws Exception { + courseTestService.testCreateAndUpdateCourseWithRemoveCourseImageOnUpdate(); + } + + @Test + @WithMockUser(username = "admin", roles = "ADMIN") + public void testCreateAndUpdateCourseWithSetNewImageDespiteRemoval() throws Exception { + courseTestService.testCreateAndUpdateCourseWithSetNewImageDespiteRemoval(); + } + @Test @WithMockUser(username = "student1", roles = "USER") void testGetCourseWithoutPermission() throws Exception { diff --git a/src/test/java/de/tum/in/www1/artemis/CourseGitlabJenkinsIntegrationTest.java b/src/test/java/de/tum/in/www1/artemis/CourseGitlabJenkinsIntegrationTest.java index 877889798d33..bf12c481f4b8 100644 --- a/src/test/java/de/tum/in/www1/artemis/CourseGitlabJenkinsIntegrationTest.java +++ b/src/test/java/de/tum/in/www1/artemis/CourseGitlabJenkinsIntegrationTest.java @@ -164,6 +164,30 @@ void testUpdateCourseGroups() throws Exception { courseTestService.testUpdateCourseGroups(); } + @Test + @WithMockUser(username = "admin", roles = "ADMIN") + public void testCreateAndUpdateCourseWithCourseImage() throws Exception { + courseTestService.testCreateAndUpdateCourseWithCourseImage(); + } + + @Test + @WithMockUser(username = "admin", roles = "ADMIN") + public void testCreateAndUpdateCourseWithPersistentCourseImageOnUpdate() throws Exception { + courseTestService.testCreateAndUpdateCourseWithPersistentCourseImageOnUpdate(); + } + + @Test + @WithMockUser(username = "admin", roles = "ADMIN") + public void testCreateAndUpdateCourseWithRemoveCourseImageOnUpdate() throws Exception { + courseTestService.testCreateAndUpdateCourseWithRemoveCourseImageOnUpdate(); + } + + @Test + @WithMockUser(username = "admin", roles = "ADMIN") + public void testCreateAndUpdateCourseWithSetNewImageDespiteRemoval() throws Exception { + courseTestService.testCreateAndUpdateCourseWithSetNewImageDespiteRemoval(); + } + @Test @WithMockUser(username = "admin", roles = "ADMIN") void testUpdateOldMembersInCourse() throws Exception { diff --git a/src/test/java/de/tum/in/www1/artemis/util/CourseTestService.java b/src/test/java/de/tum/in/www1/artemis/util/CourseTestService.java index 6c50c6846e54..b910fcc1a0b6 100644 --- a/src/test/java/de/tum/in/www1/artemis/util/CourseTestService.java +++ b/src/test/java/de/tum/in/www1/artemis/util/CourseTestService.java @@ -487,6 +487,68 @@ public void testUpdateCourseGroups() throws Exception { assertThat(updatedCourse.getTeachingAssistantGroupName()).isEqualTo("new-ta-group"); } + // Test + public void testCreateAndUpdateCourseWithCourseImage() throws Exception { + var createdCourse = createCourseWithCourseImageAndReturn(); + var courseIcon = createdCourse.getCourseIcon(); + createdCourse.setDescription("new description"); // do additional update + + // Update course + request.getMvc().perform(buildUpdateCourse(createdCourse.getId(), createdCourse, "newTestIcon")).andExpect(status().isOk()); + + List updatedCourses = courseRepo.findAll(); + assertThat(updatedCourses).hasSize(1); + var updatedCourse = updatedCourses.get(0); + assertThat(updatedCourse.getId()).isEqualTo(createdCourse.getId()); + assertThat(updatedCourse.getCourseIcon()).isNotEqualTo(courseIcon).isNotNull(); + assertThat(updatedCourse.getDescription()).isEqualTo("new description"); + } + + // Test + public void testCreateAndUpdateCourseWithPersistentCourseImageOnUpdate() throws Exception { + Course createdCourse = createCourseWithCourseImageAndReturn(); + + // Update course + request.getMvc().perform(buildUpdateCourse(createdCourse.getId(), createdCourse)).andExpect(status().isOk()); + + List updatedCourses = courseRepo.findAll(); + assertThat(updatedCourses).hasSize(1); + var updatedCourse = updatedCourses.get(0); + assertThat(updatedCourse.getId()).isEqualTo(createdCourse.getId()); + assertThat(updatedCourse.getCourseIcon()).isEqualTo(createdCourse.getCourseIcon()); + } + + // Test + public void testCreateAndUpdateCourseWithRemoveCourseImageOnUpdate() throws Exception { + Course createdCourse = createCourseWithCourseImageAndReturn(); + createdCourse.setCourseIcon(null); + + // Update course + request.getMvc().perform(buildUpdateCourse(createdCourse.getId(), createdCourse)).andExpect(status().isOk()); + + List updatedCourses = courseRepo.findAll(); + assertThat(updatedCourses).hasSize(1); + var updatedCourse = updatedCourses.get(0); + assertThat(updatedCourse.getId()).isEqualTo(createdCourse.getId()); + assertThat(updatedCourse.getCourseIcon()).isNull(); + } + + // Test + public void testCreateAndUpdateCourseWithSetNewImageDespiteRemoval() throws Exception { + Course createdCourse = createCourseWithCourseImageAndReturn(); + var courseIcon = createdCourse.getCourseIcon(); + createdCourse.setCourseIcon(null); + + // Update course + request.getMvc().perform(buildUpdateCourse(createdCourse.getId(), createdCourse, "newTestIcon")).andExpect(status().isOk()); + + List updatedCourses = courseRepo.findAll(); + assertThat(updatedCourses).hasSize(1); + var updatedCourse = updatedCourses.get(0); + assertThat(updatedCourse.getId()).isEqualTo(createdCourse.getId()); + assertThat(updatedCourse.getCourseIcon()).isNotNull().isNotEqualTo(courseIcon); + } + // Test public void testUpdateCourseGroups_InExternalCiUserManagement_failToRemoveUser() throws Exception { Course course = database.addCourseWithOneProgrammingExercise(); @@ -2207,7 +2269,7 @@ public MockHttpServletRequestBuilder buildCreateCourse(@NotNull Course course, S var coursePart = new MockMultipartFile("course", "", MediaType.APPLICATION_JSON_VALUE, objectMapper.writeValueAsString(course).getBytes()); var builder = MockMvcRequestBuilders.multipart(HttpMethod.POST, "/api/courses").file(coursePart); if (fileContent != null) { - var filePart = new MockMultipartFile("file", "test.zip", MediaType.APPLICATION_OCTET_STREAM_VALUE, fileContent.getBytes()); + var filePart = new MockMultipartFile("file", "placeholderName.png", MediaType.IMAGE_PNG_VALUE, fileContent.getBytes()); builder.file(filePart); } return builder.contentType(MediaType.MULTIPART_FORM_DATA_VALUE); @@ -2221,9 +2283,26 @@ public MockHttpServletRequestBuilder buildUpdateCourse(long id, @NotNull Course var coursePart = new MockMultipartFile("course", "", MediaType.APPLICATION_JSON_VALUE, objectMapper.writeValueAsString(course).getBytes()); var builder = MockMvcRequestBuilders.multipart(HttpMethod.PUT, "/api/courses/" + id).file(coursePart); if (fileContent != null) { - var filePart = new MockMultipartFile("file", "test.zip", MediaType.APPLICATION_OCTET_STREAM_VALUE, fileContent.getBytes()); + var filePart = new MockMultipartFile("file", "placeholderName.png", MediaType.IMAGE_PNG_VALUE, fileContent.getBytes()); builder.file(filePart); } return builder.contentType(MediaType.MULTIPART_FORM_DATA_VALUE); } + + private Course createCourseWithCourseImageAndReturn() throws Exception { + Course course = ModelFactory.generateCourse(null, null, null, new HashSet<>()); + mockDelegate.mockCreateGroupInUserManagement(course.getDefaultStudentGroupName()); + mockDelegate.mockCreateGroupInUserManagement(course.getDefaultTeachingAssistantGroupName()); + mockDelegate.mockCreateGroupInUserManagement(course.getDefaultEditorGroupName()); + mockDelegate.mockCreateGroupInUserManagement(course.getDefaultInstructorGroupName()); + + request.getMvc().perform(buildCreateCourse(course, "testIcon")).andExpect(status().isCreated()); + + List courses = courseRepo.findAll(); + assertThat(courses).as("Course got stored").hasSize(1); + var createdCourse = courses.get(0); + assertThat(createdCourse.getCourseIcon()).as("Course icon got stored").isNotNull(); + + return createdCourse; + } } From 4190b92a0299546906efdc937e6498da7c44c703 Mon Sep 17 00:00:00 2001 From: Julian Christl Date: Thu, 20 Oct 2022 16:11:08 +0200 Subject: [PATCH 09/16] CR for Cypress --- src/test/cypress/integration/CourseManagement.spec.ts | 7 +++---- src/test/cypress/integration/exam/ExamAssessment.spec.ts | 4 ++-- .../cypress/integration/exam/ExamCreationDeletion.spec.ts | 4 ++-- .../cypress/integration/exam/ExamDateVerification.spec.ts | 4 ++-- src/test/cypress/integration/exam/ExamManagement.spec.ts | 4 ++-- .../cypress/integration/exam/ExamParticipation.spec.ts | 4 ++-- .../exercises/modeling/ModelingExerciseAssessment.spec.ts | 4 ++-- .../exercises/modeling/ModelingExerciseManagement.spec.ts | 4 ++-- .../modeling/ModelingExerciseParticipation.spec.ts | 4 ++-- .../programming/ProgrammingExerciseAssessment.spec.ts | 4 ++-- .../programming/ProgrammingExerciseManagement.spec.ts | 4 ++-- .../programming/ProgrammingExerciseParticipation.spec.ts | 4 ++-- .../ProgrammingExerciseStaticCodeAnalysis.spec.ts | 4 ++-- .../exercises/quiz-exercise/QuizExerciseAssessment.spec.ts | 4 ++-- .../exercises/quiz-exercise/QuizExerciseManagement.spec.ts | 4 ++-- .../quiz-exercise/QuizExerciseParticipation.spec.ts | 4 ++-- .../exercises/text/TextExerciseAssessment.spec.ts | 4 ++-- .../exercises/text/TextExerciseManagement.spec.ts | 4 ++-- .../exercises/text/TextExerciseParticipation.spec.ts | 4 ++-- .../cypress/integration/lecture/LectureManagement.spec.ts | 4 ++-- .../cypress/support/requests/CourseManagementRequests.ts | 2 +- 21 files changed, 42 insertions(+), 43 deletions(-) diff --git a/src/test/cypress/integration/CourseManagement.spec.ts b/src/test/cypress/integration/CourseManagement.spec.ts index 85be00a8a8b2..0ea6fea65df0 100644 --- a/src/test/cypress/integration/CourseManagement.spec.ts +++ b/src/test/cypress/integration/CourseManagement.spec.ts @@ -1,13 +1,12 @@ import { Interception } from 'cypress/types/net-stubbing'; -import { COURSE_BASE, parseCourseAfterMultiPart } from '../support/requests/CourseManagementRequests'; +import { COURSE_BASE, convertCourseAfterMultiPart } from '../support/requests/CourseManagementRequests'; import { GET, BASE_API, POST } from '../support/constants'; import { artemis } from '../support/ArtemisTesting'; import { CourseManagementPage } from '../support/pageobjects/course/CourseManagementPage'; import { NavigationBar } from '../support/pageobjects/NavigationBar'; import { ArtemisRequests } from '../support/requests/ArtemisRequests'; -import { generateUUID, parseArrayBufferAsJsonObject } from '../support/utils'; +import { generateUUID } from '../support/utils'; import { Course } from 'app/entities/course.model'; -import * as Cypress from 'cypress'; // Requests const artemisRequests: ArtemisRequests = new ArtemisRequests(); @@ -42,7 +41,7 @@ describe('Course management', () => { beforeEach(() => { artemisRequests.courseManagement.createCourse(false, courseName, courseShortName).then((response) => { - course = parseCourseAfterMultiPart(response); + course = convertCourseAfterMultiPart(response); courseId = course.id!; }); }); diff --git a/src/test/cypress/integration/exam/ExamAssessment.spec.ts b/src/test/cypress/integration/exam/ExamAssessment.spec.ts index 3e5027b42c67..3207da7b1fc7 100644 --- a/src/test/cypress/integration/exam/ExamAssessment.spec.ts +++ b/src/test/cypress/integration/exam/ExamAssessment.spec.ts @@ -3,7 +3,7 @@ import { Course } from 'app/entities/course.model'; import { ExerciseGroup } from 'app/entities/exercise-group.model'; import { Exam } from 'app/entities/exam.model'; import { artemis } from '../../support/ArtemisTesting'; -import { CypressAssessmentType, CypressExamBuilder, parseCourseAfterMultiPart } from '../../support/requests/CourseManagementRequests'; +import { CypressAssessmentType, CypressExamBuilder, convertCourseAfterMultiPart } from '../../support/requests/CourseManagementRequests'; import partiallySuccessful from '../../fixtures/programming_exercise_submissions/partially_successful/submission.json'; import dayjs, { Dayjs } from 'dayjs/esm'; import textSubmission from '../../fixtures/text_exercise_submission/text_exercise_submission.json'; @@ -45,7 +45,7 @@ describe('Exam assessment', () => { before('Create a course', () => { cy.login(admin); courseManagementRequests.createCourse(true).then((response) => { - course = parseCourseAfterMultiPart(response); + course = convertCourseAfterMultiPart(response); courseManagementRequests.addStudentToCourse(course, artemis.users.getStudentOne()); courseManagementRequests.addTutorToCourse(course, artemis.users.getTutor()); }); diff --git a/src/test/cypress/integration/exam/ExamCreationDeletion.spec.ts b/src/test/cypress/integration/exam/ExamCreationDeletion.spec.ts index 2dd5e7bb7c60..5f17757d0911 100644 --- a/src/test/cypress/integration/exam/ExamCreationDeletion.spec.ts +++ b/src/test/cypress/integration/exam/ExamCreationDeletion.spec.ts @@ -1,6 +1,6 @@ import { Interception } from 'cypress/types/net-stubbing'; import { Course } from 'app/entities/course.model'; -import { CypressExamBuilder, parseCourseAfterMultiPart } from '../../support/requests/CourseManagementRequests'; +import { CypressExamBuilder, convertCourseAfterMultiPart } from '../../support/requests/CourseManagementRequests'; import dayjs from 'dayjs/esm'; import { artemis } from '../../support/ArtemisTesting'; import { generateUUID } from '../../support/utils'; @@ -23,7 +23,7 @@ describe('Exam creation/deletion', () => { before(() => { cy.login(artemis.users.getAdmin()); courseManagementRequests.createCourse().then((response) => { - course = parseCourseAfterMultiPart(response); + course = convertCourseAfterMultiPart(response); }); }); diff --git a/src/test/cypress/integration/exam/ExamDateVerification.spec.ts b/src/test/cypress/integration/exam/ExamDateVerification.spec.ts index 4efaad39f087..64d57d2836d3 100644 --- a/src/test/cypress/integration/exam/ExamDateVerification.spec.ts +++ b/src/test/cypress/integration/exam/ExamDateVerification.spec.ts @@ -1,7 +1,7 @@ import { ExerciseGroup } from 'app/entities/exercise-group.model'; import { Course } from 'app/entities/course.model'; import { Exam } from 'app/entities/exam.model'; -import { CypressExamBuilder, parseCourseAfterMultiPart } from '../../support/requests/CourseManagementRequests'; +import { CypressExamBuilder, convertCourseAfterMultiPart } from '../../support/requests/CourseManagementRequests'; import dayjs from 'dayjs/esm'; import { artemis } from '../../support/ArtemisTesting'; import { generateUUID } from '../../support/utils'; @@ -22,7 +22,7 @@ describe('Exam date verification', () => { before(() => { cy.login(artemis.users.getAdmin()); courseManagementRequests.createCourse().then((response) => { - course = parseCourseAfterMultiPart(response); + course = convertCourseAfterMultiPart(response); courseManagementRequests.addStudentToCourse(course, artemis.users.getStudentOne()); }); }); diff --git a/src/test/cypress/integration/exam/ExamManagement.spec.ts b/src/test/cypress/integration/exam/ExamManagement.spec.ts index b4440cdf8813..037aca2d151b 100644 --- a/src/test/cypress/integration/exam/ExamManagement.spec.ts +++ b/src/test/cypress/integration/exam/ExamManagement.spec.ts @@ -1,7 +1,7 @@ import { Interception } from 'cypress/types/net-stubbing'; import { Exam } from 'app/entities/exam.model'; import { Course } from 'app/entities/course.model'; -import { CypressExamBuilder, parseCourseAfterMultiPart } from '../../support/requests/CourseManagementRequests'; +import { CypressExamBuilder, convertCourseAfterMultiPart } from '../../support/requests/CourseManagementRequests'; import { artemis } from '../../support/ArtemisTesting'; import { generateUUID } from '../../support/utils'; @@ -31,7 +31,7 @@ describe('Exam management', () => { before(() => { cy.login(users.getAdmin()); courseManagementRequests.createCourse().then((response) => { - course = parseCourseAfterMultiPart(response); + course = convertCourseAfterMultiPart(response); courseManagementRequests.addStudentToCourse(course, users.getStudentOne()); const examConfig = new CypressExamBuilder(course).title(examTitle).build(); courseManagementRequests.createExam(examConfig).then((examResponse) => { diff --git a/src/test/cypress/integration/exam/ExamParticipation.spec.ts b/src/test/cypress/integration/exam/ExamParticipation.spec.ts index 42a61d91676b..1fa6d21a6f7b 100644 --- a/src/test/cypress/integration/exam/ExamParticipation.spec.ts +++ b/src/test/cypress/integration/exam/ExamParticipation.spec.ts @@ -1,7 +1,7 @@ import { QuizExercise } from 'app/entities/quiz/quiz-exercise.model'; import { Exam } from 'app/entities/exam.model'; import { GET, BASE_API } from '../../support/constants'; -import { CypressExamBuilder, parseCourseAfterMultiPart } from '../../support/requests/CourseManagementRequests'; +import { CypressExamBuilder, convertCourseAfterMultiPart } from '../../support/requests/CourseManagementRequests'; import { artemis } from '../../support/ArtemisTesting'; import dayjs from 'dayjs/esm'; import submission from '../../fixtures/programming_exercise_submissions/all_successful/submission.json'; @@ -37,7 +37,7 @@ describe('Exam participation', () => { before(() => { cy.login(users.getAdmin()); courseRequests.createCourse(true).then((response) => { - course = parseCourseAfterMultiPart(response); + course = convertCourseAfterMultiPart(response); const examContent = new CypressExamBuilder(course) .visibleDate(dayjs().subtract(3, 'days')) .startDate(dayjs().subtract(2, 'days')) diff --git a/src/test/cypress/integration/exercises/modeling/ModelingExerciseAssessment.spec.ts b/src/test/cypress/integration/exercises/modeling/ModelingExerciseAssessment.spec.ts index f13f814c491e..f9dfd5eaa985 100644 --- a/src/test/cypress/integration/exercises/modeling/ModelingExerciseAssessment.spec.ts +++ b/src/test/cypress/integration/exercises/modeling/ModelingExerciseAssessment.spec.ts @@ -2,7 +2,7 @@ import { ModelingExercise } from 'app/entities/modeling-exercise.model'; import { Course } from 'app/entities/course.model'; import { artemis } from '../../../support/ArtemisTesting'; import day from 'dayjs/esm'; -import { parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; +import { convertCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; // pageobjects const assessmentEditor = artemis.pageobjects.assessment.modeling; @@ -86,7 +86,7 @@ describe('Modeling Exercise Assessment Spec', () => { function createCourseWithModelingExercise() { cy.login(admin); return courseManagementRequests.createCourse(true).then((response) => { - course = parseCourseAfterMultiPart(response); + course = convertCourseAfterMultiPart(response); courseManagementRequests.addStudentToCourse(course, student); courseManagementRequests.addTutorToCourse(course, tutor); courseManagementRequests.addInstructorToCourse(course, instructor); diff --git a/src/test/cypress/integration/exercises/modeling/ModelingExerciseManagement.spec.ts b/src/test/cypress/integration/exercises/modeling/ModelingExerciseManagement.spec.ts index ce45d7683059..075f5468a79e 100644 --- a/src/test/cypress/integration/exercises/modeling/ModelingExerciseManagement.spec.ts +++ b/src/test/cypress/integration/exercises/modeling/ModelingExerciseManagement.spec.ts @@ -6,7 +6,7 @@ import { MODELING_EDITOR_CANVAS } from '../../../support/pageobjects/exercises/m // https://day.js.org/docs is a tool for date/time import dayjs from 'dayjs/esm'; -import { parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; +import { convertCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; // pageobjects const createModelingExercise = artemis.pageobjects.exercise.modeling.creation; @@ -29,7 +29,7 @@ describe('Modeling Exercise Management Spec', () => { before('Create a course', () => { cy.login(admin); courseManagementRequests.createCourse().then((response: Cypress.Response) => { - course = parseCourseAfterMultiPart(response); + course = convertCourseAfterMultiPart(response); courseManagementRequests.addInstructorToCourse(course, instructor); courseManagementRequests.addStudentToCourse(course, student); }); diff --git a/src/test/cypress/integration/exercises/modeling/ModelingExerciseParticipation.spec.ts b/src/test/cypress/integration/exercises/modeling/ModelingExerciseParticipation.spec.ts index 720a41b17ad5..04da26865a61 100644 --- a/src/test/cypress/integration/exercises/modeling/ModelingExerciseParticipation.spec.ts +++ b/src/test/cypress/integration/exercises/modeling/ModelingExerciseParticipation.spec.ts @@ -1,7 +1,7 @@ import { ModelingExercise } from 'app/entities/modeling-exercise.model'; import { Course } from 'app/entities/course.model'; import { artemis } from '../../../support/ArtemisTesting'; -import { parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; +import { convertCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; // pageobjects const courseManagement = artemis.pageobjects.course.management; @@ -20,7 +20,7 @@ describe('Modeling Exercise Participation Spec', () => { before('Log in as admin and create a course', () => { cy.login(admin); courseManagementRequests.createCourse().then((response: Cypress.Response) => { - course = parseCourseAfterMultiPart(response); + course = convertCourseAfterMultiPart(response); cy.visit(`/course-management/${course.id}`); courseManagement.addStudentToCourse(student); courseManagementRequests.createModelingExercise({ course }).then((resp: Cypress.Response) => { diff --git a/src/test/cypress/integration/exercises/programming/ProgrammingExerciseAssessment.spec.ts b/src/test/cypress/integration/exercises/programming/ProgrammingExerciseAssessment.spec.ts index c00a2261ee34..45a3fd0d420b 100644 --- a/src/test/cypress/integration/exercises/programming/ProgrammingExerciseAssessment.spec.ts +++ b/src/test/cypress/integration/exercises/programming/ProgrammingExerciseAssessment.spec.ts @@ -1,7 +1,7 @@ import { Interception } from 'cypress/types/net-stubbing'; import { ProgrammingExercise } from 'app/entities/programming-exercise.model'; import { Course } from 'app/entities/course.model'; -import { CypressAssessmentType, parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; +import { CypressAssessmentType, convertCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; import { artemis } from 'src/test/cypress/support/ArtemisTesting'; import dayjs from 'dayjs/esm'; @@ -96,7 +96,7 @@ describe('Programming exercise assessment', () => { function createCourseWithProgrammingExercise() { cy.login(admin); return courseManagement.createCourse(true).then((response) => { - course = parseCourseAfterMultiPart(response); + course = convertCourseAfterMultiPart(response); courseManagement.addStudentToCourse(course, student); courseManagement.addTutorToCourse(course, tutor); courseManagement.addInstructorToCourse(course, instructor); diff --git a/src/test/cypress/integration/exercises/programming/ProgrammingExerciseManagement.spec.ts b/src/test/cypress/integration/exercises/programming/ProgrammingExerciseManagement.spec.ts index cd9dffa60326..02954ce353c6 100644 --- a/src/test/cypress/integration/exercises/programming/ProgrammingExerciseManagement.spec.ts +++ b/src/test/cypress/integration/exercises/programming/ProgrammingExerciseManagement.spec.ts @@ -4,7 +4,7 @@ import { Course } from 'app/entities/course.model'; import { DELETE } from '../../../support/constants'; import { artemis } from '../../../support/ArtemisTesting'; import { generateUUID } from '../../../support/utils'; -import { parseCourseAfterMultiPart, PROGRAMMING_EXERCISE_BASE } from '../../../support/requests/CourseManagementRequests'; +import { convertCourseAfterMultiPart, PROGRAMMING_EXERCISE_BASE } from '../../../support/requests/CourseManagementRequests'; // Admin account const admin = artemis.users.getAdmin(); @@ -24,7 +24,7 @@ describe('Programming Exercise Management', () => { before(() => { cy.login(admin); artemisRequests.courseManagement.createCourse(true).then((response) => { - course = parseCourseAfterMultiPart(response); + course = convertCourseAfterMultiPart(response); expect(course).property('id').to.be.a('number'); }); }); diff --git a/src/test/cypress/integration/exercises/programming/ProgrammingExerciseParticipation.spec.ts b/src/test/cypress/integration/exercises/programming/ProgrammingExerciseParticipation.spec.ts index 952f7bbe4bec..f3fb2ba95a25 100644 --- a/src/test/cypress/integration/exercises/programming/ProgrammingExerciseParticipation.spec.ts +++ b/src/test/cypress/integration/exercises/programming/ProgrammingExerciseParticipation.spec.ts @@ -4,7 +4,7 @@ import allSuccessful from '../../../fixtures/programming_exercise_submissions/al import partiallySuccessful from '../../../fixtures/programming_exercise_submissions/partially_successful/submission.json'; import { artemis } from '../../../support/ArtemisTesting'; import { makeSubmissionAndVerifyResults, startParticipationInProgrammingExercise } from '../../../support/pageobjects/exercises/programming/OnlineEditorPage'; -import { parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; +import { convertCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; // The user management object const users = artemis.users; @@ -51,7 +51,7 @@ describe('Programming exercise participations', () => { function setupCourseAndProgrammingExercise() { cy.login(users.getAdmin(), '/'); courseManagement.createCourse(true).then((response) => { - course = parseCourseAfterMultiPart(response); + course = convertCourseAfterMultiPart(response); courseManagement.addStudentToCourse(course, users.getStudentOne()); courseManagement.addStudentToCourse(course, users.getStudentTwo()); courseManagement.addStudentToCourse(course, users.getStudentThree()); diff --git a/src/test/cypress/integration/exercises/programming/ProgrammingExerciseStaticCodeAnalysis.spec.ts b/src/test/cypress/integration/exercises/programming/ProgrammingExerciseStaticCodeAnalysis.spec.ts index 69c6dabff5d1..75b9f2e89672 100644 --- a/src/test/cypress/integration/exercises/programming/ProgrammingExerciseStaticCodeAnalysis.spec.ts +++ b/src/test/cypress/integration/exercises/programming/ProgrammingExerciseStaticCodeAnalysis.spec.ts @@ -3,7 +3,7 @@ import { Course } from 'app/entities/course.model'; import scaSubmission from '../../../fixtures/programming_exercise_submissions/static_code_analysis/submission.json'; import { artemis } from '../../../support/ArtemisTesting'; import { makeSubmissionAndVerifyResults, startParticipationInProgrammingExercise } from '../../../support/pageobjects/exercises/programming/OnlineEditorPage'; -import { parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; +import { convertCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; // The user management object const users = artemis.users; @@ -43,7 +43,7 @@ describe('Static code analysis tests', () => { function setupCourseAndProgrammingExercise() { cy.login(users.getAdmin()); courseManagement.createCourse(true).then((response) => { - course = parseCourseAfterMultiPart(response); + course = convertCourseAfterMultiPart(response); courseManagement.addStudentToCourse(course, users.getStudentOne()); courseManagement.createProgrammingExercise({ course }, 50).then((dto) => { exercise = dto.body; diff --git a/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseAssessment.spec.ts b/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseAssessment.spec.ts index c1ed1586ca69..4b8be4ac207a 100644 --- a/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseAssessment.spec.ts +++ b/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseAssessment.spec.ts @@ -3,7 +3,7 @@ import { Course } from 'app/entities/course.model'; import { artemis } from '../../../support/ArtemisTesting'; import shortAnswerQuizTemplate from '../../../fixtures/quiz_exercise_fixtures/shortAnswerQuiz_template.json'; import multipleChoiceQuizTemplate from '../../../fixtures/quiz_exercise_fixtures/multipleChoiceQuiz_template.json'; -import { parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; +import { convertCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; // Accounts const admin = artemis.users.getAdmin(); @@ -23,7 +23,7 @@ describe('Quiz Exercise Assessment', () => { before('Set up course', () => { cy.login(admin); courseManagementRequest.createCourse().then((response) => { - course = parseCourseAfterMultiPart(response); + course = convertCourseAfterMultiPart(response); courseManagementRequest.addStudentToCourse(course, student); courseManagementRequest.addTutorToCourse(course, tutor); }); diff --git a/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseManagement.spec.ts b/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseManagement.spec.ts index 04e65d5a5572..6b7b7a0fda0f 100644 --- a/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseManagement.spec.ts +++ b/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseManagement.spec.ts @@ -5,7 +5,7 @@ import { artemis } from '../../../support/ArtemisTesting'; import { generateUUID } from '../../../support/utils'; import multipleChoiceTemplate from '../../../fixtures/quiz_exercise_fixtures/multipleChoiceQuiz_template.json'; import { DELETE } from '../../../support/constants'; -import { parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; +import { convertCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; // Accounts const admin = artemis.users.getAdmin(); @@ -25,7 +25,7 @@ describe('Quiz Exercise Management', () => { before('Set up course', () => { cy.login(admin); courseManagementRequest.createCourse().then((response) => { - course = parseCourseAfterMultiPart(response); + course = convertCourseAfterMultiPart(response); }); }); diff --git a/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseParticipation.spec.ts b/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseParticipation.spec.ts index f7193aadcb2a..950be97d0a58 100644 --- a/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseParticipation.spec.ts +++ b/src/test/cypress/integration/exercises/quiz-exercise/QuizExerciseParticipation.spec.ts @@ -3,7 +3,7 @@ import { Course } from 'app/entities/course.model'; import { artemis } from '../../../support/ArtemisTesting'; import multipleChoiceQuizTemplate from '../../../fixtures/quiz_exercise_fixtures/multipleChoiceQuiz_template.json'; import shortAnswerQuizTemplate from '../../../fixtures/quiz_exercise_fixtures/shortAnswerQuiz_template.json'; -import { parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; +import { convertCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; // Accounts const admin = artemis.users.getAdmin(); @@ -27,7 +27,7 @@ describe('Quiz Exercise Participation', () => { before('Set up course', () => { cy.login(admin); courseManagementRequest.createCourse().then((response) => { - course = parseCourseAfterMultiPart(response); + course = convertCourseAfterMultiPart(response); courseManagementRequest.addStudentToCourse(course, student); }); }); diff --git a/src/test/cypress/integration/exercises/text/TextExerciseAssessment.spec.ts b/src/test/cypress/integration/exercises/text/TextExerciseAssessment.spec.ts index dd38f1c0ba41..5bc13345e88f 100644 --- a/src/test/cypress/integration/exercises/text/TextExerciseAssessment.spec.ts +++ b/src/test/cypress/integration/exercises/text/TextExerciseAssessment.spec.ts @@ -2,7 +2,7 @@ import { Interception } from 'cypress/types/net-stubbing'; import { TextExercise } from 'app/entities/text-exercise.model'; import { Course } from 'app/entities/course.model'; import { artemis } from 'src/test/cypress/support/ArtemisTesting'; -import { parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; +import { convertCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; // The user management object const users = artemis.users; @@ -88,7 +88,7 @@ describe('Text exercise assessment', () => { function createCourseWithTextExercise() { cy.login(admin); return courseManagement.createCourse().then((response) => { - course = parseCourseAfterMultiPart(response); + course = convertCourseAfterMultiPart(response); courseManagement.addStudentToCourse(course, student); courseManagement.addTutorToCourse(course, tutor); courseManagement.addInstructorToCourse(course, instructor); diff --git a/src/test/cypress/integration/exercises/text/TextExerciseManagement.spec.ts b/src/test/cypress/integration/exercises/text/TextExerciseManagement.spec.ts index 195975bc014b..b3d6b7093158 100644 --- a/src/test/cypress/integration/exercises/text/TextExerciseManagement.spec.ts +++ b/src/test/cypress/integration/exercises/text/TextExerciseManagement.spec.ts @@ -6,7 +6,7 @@ import { DELETE } from '../../../support/constants'; import { generateUUID } from '../../../support/utils'; import { artemis } from '../../../support/ArtemisTesting'; import dayjs from 'dayjs/esm'; -import { parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; +import { convertCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; // The user management object const users = artemis.users; @@ -28,7 +28,7 @@ describe('Text exercise management', () => { before(() => { cy.login(users.getAdmin()); courseManagement.createCourse().then((response) => { - course = parseCourseAfterMultiPart(response); + course = convertCourseAfterMultiPart(response); }); }); diff --git a/src/test/cypress/integration/exercises/text/TextExerciseParticipation.spec.ts b/src/test/cypress/integration/exercises/text/TextExerciseParticipation.spec.ts index 5cdf79db9f00..ce4b4c5b62f8 100644 --- a/src/test/cypress/integration/exercises/text/TextExerciseParticipation.spec.ts +++ b/src/test/cypress/integration/exercises/text/TextExerciseParticipation.spec.ts @@ -2,7 +2,7 @@ import { Interception } from 'cypress/types/net-stubbing'; import { TextExercise } from 'app/entities/text-exercise.model'; import { Course } from 'app/entities/course.model'; import { artemis } from '../../../support/ArtemisTesting'; -import { parseCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; +import { convertCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; // The user management object const users = artemis.users; @@ -21,7 +21,7 @@ describe('Text exercise participation', () => { before(() => { cy.login(users.getAdmin()); courseManagement.createCourse().then((response) => { - course = parseCourseAfterMultiPart(response); + course = convertCourseAfterMultiPart(response); courseManagement.addStudentToCourse(course, users.getStudentOne()); courseManagement.createTextExercise({ course }).then((exerciseResponse: Cypress.Response) => { exercise = exerciseResponse.body; diff --git a/src/test/cypress/integration/lecture/LectureManagement.spec.ts b/src/test/cypress/integration/lecture/LectureManagement.spec.ts index 7d544c62f665..a509e8565d78 100644 --- a/src/test/cypress/integration/lecture/LectureManagement.spec.ts +++ b/src/test/cypress/integration/lecture/LectureManagement.spec.ts @@ -3,7 +3,7 @@ import { Course } from 'app/entities/course.model'; import { artemis } from '../../support/ArtemisTesting'; import { generateUUID } from '../../support/utils'; import dayjs from 'dayjs/esm'; -import { parseCourseAfterMultiPart } from '../../support/requests/CourseManagementRequests'; +import { convertCourseAfterMultiPart } from '../../support/requests/CourseManagementRequests'; // Requests const courseManagementRequests = artemis.requests.courseManagement; @@ -23,7 +23,7 @@ describe('Lecture management', () => { before(() => { cy.login(admin); courseManagementRequests.createCourse().then((response) => { - course = parseCourseAfterMultiPart(response); + course = convertCourseAfterMultiPart(response); courseManagementRequests.addInstructorToCourse(course, instructor); }); }); diff --git a/src/test/cypress/support/requests/CourseManagementRequests.ts b/src/test/cypress/support/requests/CourseManagementRequests.ts index 6fa06636222d..928aee6ae959 100644 --- a/src/test/cypress/support/requests/CourseManagementRequests.ts +++ b/src/test/cypress/support/requests/CourseManagementRequests.ts @@ -634,7 +634,7 @@ export enum CypressExerciseType { QUIZ, } -export function parseCourseAfterMultiPart(response: Cypress.Response): Course { +export function convertCourseAfterMultiPart(response: Cypress.Response): Course { // Cypress currently has some issues with our multipart request, parsing this not as an object but as an ArrayBuffer // Once this is fixed (and hence the expect statements below fail), we can remove the additional parsing expect(response.body).not.to.be.an('object'); From 3136308aaf88ae956c38b8e0cd00e7fea0363863 Mon Sep 17 00:00:00 2001 From: Julian Christl Date: Thu, 20 Oct 2022 16:12:02 +0200 Subject: [PATCH 10/16] more CR --- .../java/de/tum/in/www1/artemis/web/rest/CourseResource.java | 3 +-- src/main/webapp/app/course/manage/course-update.component.ts | 2 +- .../shared/image-cropper/component/image-cropper.component.ts | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) 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 23c46274ce52..3e0670529fcd 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 @@ -453,8 +453,7 @@ public ResponseEntity getCourseForAssessmentDashboard(@PathVariable long /** * GET /courses/:courseId/stats-for-assessment-dashboard A collection of useful statistics for the tutor course dashboard, including: - number of submissions to the course - - * number of - * assessments - number of assessments assessed by the tutor - number of complaints + * number of assessments - number of assessments assessed by the tutor - number of complaints *

* all timestamps were measured when calling this method from the PGdP assessment-dashboard * diff --git a/src/main/webapp/app/course/manage/course-update.component.ts b/src/main/webapp/app/course/manage/course-update.component.ts index f7e49005eb6a..ecaacab65923 100644 --- a/src/main/webapp/app/course/manage/course-update.component.ts +++ b/src/main/webapp/app/course/manage/course-update.component.ts @@ -261,7 +261,7 @@ export class CourseUpdateComponent implements OnInit { */ setCourseImage(event: Event): void { const element = event.currentTarget as HTMLInputElement; - if (element.files && element.files[0]) { + if (element.files?.[0]) { this.courseImageUploadFile = element.files[0]; } } diff --git a/src/main/webapp/app/shared/image-cropper/component/image-cropper.component.ts b/src/main/webapp/app/shared/image-cropper/component/image-cropper.component.ts index edc741420fc9..a1c34c7d8889 100644 --- a/src/main/webapp/app/shared/image-cropper/component/image-cropper.component.ts +++ b/src/main/webapp/app/shared/image-cropper/component/image-cropper.component.ts @@ -178,7 +178,7 @@ export class ImageCropperComponent implements OnChanges, OnInit { } const element = this.imageChangedEvent.currentTarget as HTMLInputElement; - return !!element.files && element.files.length > 0; + return !!element.files?.length; } private setCssTransform() { From ff461891d95382fe76d8e125f4483f93d44470ad Mon Sep 17 00:00:00 2001 From: Julian Christl Date: Thu, 20 Oct 2022 16:48:11 +0200 Subject: [PATCH 11/16] replace ignore comment --- .../image-cropper/component/image-cropper.component.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/webapp/app/shared/image-cropper/component/image-cropper.component.ts b/src/main/webapp/app/shared/image-cropper/component/image-cropper.component.ts index a1c34c7d8889..b5ae79747d73 100644 --- a/src/main/webapp/app/shared/image-cropper/component/image-cropper.component.ts +++ b/src/main/webapp/app/shared/image-cropper/component/image-cropper.component.ts @@ -158,8 +158,9 @@ export class ImageCropperComponent implements OnChanges, OnInit { } if (changes.imageChangedEvent && this.isValidImageChangedEvent()) { const element = this.imageChangedEvent?.currentTarget as HTMLInputElement; - // @ts-ignore Ignore "is possibly null", checked before - this.loadImageFile(element.files[0]); + if (element.files) { + this.loadImageFile(element.files?.[0]); + } } if (changes.imageURL && this.imageURL) { this.loadImageFromURL(this.imageURL); From 267d004207f40145ce93c9bc22bd35b8d8077ac2 Mon Sep 17 00:00:00 2001 From: Julian Christl Date: Thu, 20 Oct 2022 19:34:43 +0200 Subject: [PATCH 12/16] fix codacy --- .../artemis/CourseBitbucketBambooJiraIntegrationTest.java | 8 ++++---- .../www1/artemis/CourseGitlabJenkinsIntegrationTest.java | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/test/java/de/tum/in/www1/artemis/CourseBitbucketBambooJiraIntegrationTest.java b/src/test/java/de/tum/in/www1/artemis/CourseBitbucketBambooJiraIntegrationTest.java index a716f853ae89..9cbdff959d06 100644 --- a/src/test/java/de/tum/in/www1/artemis/CourseBitbucketBambooJiraIntegrationTest.java +++ b/src/test/java/de/tum/in/www1/artemis/CourseBitbucketBambooJiraIntegrationTest.java @@ -163,25 +163,25 @@ void testUpdateCourseGroups() throws Exception { @Test @WithMockUser(username = "admin", roles = "ADMIN") - public void testCreateAndUpdateCourseWithCourseImage() throws Exception { + void testCreateAndUpdateCourseWithCourseImage() throws Exception { courseTestService.testCreateAndUpdateCourseWithCourseImage(); } @Test @WithMockUser(username = "admin", roles = "ADMIN") - public void testCreateAndUpdateCourseWithPersistentCourseImageOnUpdate() throws Exception { + void testCreateAndUpdateCourseWithPersistentCourseImageOnUpdate() throws Exception { courseTestService.testCreateAndUpdateCourseWithPersistentCourseImageOnUpdate(); } @Test @WithMockUser(username = "admin", roles = "ADMIN") - public void testCreateAndUpdateCourseWithRemoveCourseImageOnUpdate() throws Exception { + void testCreateAndUpdateCourseWithRemoveCourseImageOnUpdate() throws Exception { courseTestService.testCreateAndUpdateCourseWithRemoveCourseImageOnUpdate(); } @Test @WithMockUser(username = "admin", roles = "ADMIN") - public void testCreateAndUpdateCourseWithSetNewImageDespiteRemoval() throws Exception { + void testCreateAndUpdateCourseWithSetNewImageDespiteRemoval() throws Exception { courseTestService.testCreateAndUpdateCourseWithSetNewImageDespiteRemoval(); } diff --git a/src/test/java/de/tum/in/www1/artemis/CourseGitlabJenkinsIntegrationTest.java b/src/test/java/de/tum/in/www1/artemis/CourseGitlabJenkinsIntegrationTest.java index bf12c481f4b8..cd05c9d957fd 100644 --- a/src/test/java/de/tum/in/www1/artemis/CourseGitlabJenkinsIntegrationTest.java +++ b/src/test/java/de/tum/in/www1/artemis/CourseGitlabJenkinsIntegrationTest.java @@ -166,25 +166,25 @@ void testUpdateCourseGroups() throws Exception { @Test @WithMockUser(username = "admin", roles = "ADMIN") - public void testCreateAndUpdateCourseWithCourseImage() throws Exception { + void testCreateAndUpdateCourseWithCourseImage() throws Exception { courseTestService.testCreateAndUpdateCourseWithCourseImage(); } @Test @WithMockUser(username = "admin", roles = "ADMIN") - public void testCreateAndUpdateCourseWithPersistentCourseImageOnUpdate() throws Exception { + void testCreateAndUpdateCourseWithPersistentCourseImageOnUpdate() throws Exception { courseTestService.testCreateAndUpdateCourseWithPersistentCourseImageOnUpdate(); } @Test @WithMockUser(username = "admin", roles = "ADMIN") - public void testCreateAndUpdateCourseWithRemoveCourseImageOnUpdate() throws Exception { + void testCreateAndUpdateCourseWithRemoveCourseImageOnUpdate() throws Exception { courseTestService.testCreateAndUpdateCourseWithRemoveCourseImageOnUpdate(); } @Test @WithMockUser(username = "admin", roles = "ADMIN") - public void testCreateAndUpdateCourseWithSetNewImageDespiteRemoval() throws Exception { + void testCreateAndUpdateCourseWithSetNewImageDespiteRemoval() throws Exception { courseTestService.testCreateAndUpdateCourseWithSetNewImageDespiteRemoval(); } From 02da1714cec871d06b29b01b679ed63a4fb4e0fc Mon Sep 17 00:00:00 2001 From: Julian Christl Date: Fri, 21 Oct 2022 13:45:17 +0200 Subject: [PATCH 13/16] make Johannes happy --- .../shared/image-cropper/component/image-cropper.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/webapp/app/shared/image-cropper/component/image-cropper.component.ts b/src/main/webapp/app/shared/image-cropper/component/image-cropper.component.ts index b5ae79747d73..fc46c4006093 100644 --- a/src/main/webapp/app/shared/image-cropper/component/image-cropper.component.ts +++ b/src/main/webapp/app/shared/image-cropper/component/image-cropper.component.ts @@ -159,7 +159,7 @@ export class ImageCropperComponent implements OnChanges, OnInit { if (changes.imageChangedEvent && this.isValidImageChangedEvent()) { const element = this.imageChangedEvent?.currentTarget as HTMLInputElement; if (element.files) { - this.loadImageFile(element.files?.[0]); + this.loadImageFile(element.files[0]); } } if (changes.imageURL && this.imageURL) { From ad6d71fdd750b1319dc3e77a85a5403fdfec1a6c Mon Sep 17 00:00:00 2001 From: Julian Christl Date: Thu, 27 Oct 2022 12:10:53 +0200 Subject: [PATCH 14/16] add image cropper test --- .../component/image-cropper.component.ts | 16 +- .../image-croppper.component.spec.ts | 249 ++++++++++++++++++ 2 files changed, 257 insertions(+), 8 deletions(-) create mode 100644 src/test/javascript/spec/component/shared/image-cropper/image-croppper.component.spec.ts diff --git a/src/main/webapp/app/shared/image-cropper/component/image-cropper.component.ts b/src/main/webapp/app/shared/image-cropper/component/image-cropper.component.ts index fc46c4006093..f8d6b3fa761d 100644 --- a/src/main/webapp/app/shared/image-cropper/component/image-cropper.component.ts +++ b/src/main/webapp/app/shared/image-cropper/component/image-cropper.component.ts @@ -38,10 +38,10 @@ import { ImageCroppedEvent } from 'app/shared/image-cropper/interfaces/image-cro changeDetection: ChangeDetectionStrategy.OnPush, }) export class ImageCropperComponent implements OnChanges, OnInit { - private settings = new CropperSettings(); - private setImageMaxSizeRetries = 0; - private moveStart: MoveStart; - private loadedImage?: LoadedImage; + settings = new CropperSettings(); + setImageMaxSizeRetries = 0; + moveStart: MoveStart; + loadedImage?: LoadedImage; safeImgDataUrl: SafeUrl | string; safeTransformStyle: SafeStyle | string; @@ -108,6 +108,10 @@ export class ImageCropperComponent implements OnChanges, OnInit { this.reset(); } + ngOnInit(): void { + this.settings.stepSize = this.initialStepSize; + } + ngOnChanges(changes: SimpleChanges): void { this.onChangesUpdateSettings(changes); this.onChangesInputImage(changes); @@ -196,10 +200,6 @@ export class ImageCropperComponent implements OnChanges, OnInit { ); } - ngOnInit(): void { - this.settings.stepSize = this.initialStepSize; - } - private reset(): void { this.imageVisible = false; this.loadedImage = undefined; diff --git a/src/test/javascript/spec/component/shared/image-cropper/image-croppper.component.spec.ts b/src/test/javascript/spec/component/shared/image-cropper/image-croppper.component.spec.ts new file mode 100644 index 000000000000..01df0eaf975e --- /dev/null +++ b/src/test/javascript/spec/component/shared/image-cropper/image-croppper.component.spec.ts @@ -0,0 +1,249 @@ +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { ImageCropperComponent } from 'app/shared/image-cropper/component/image-cropper.component'; +import { CropperPositionService } from 'app/shared/image-cropper/services/cropper-position.service'; +import { CropService } from 'app/shared/image-cropper/services/crop.service'; +import { LoadedImage } from 'app/shared/image-cropper/interfaces/loaded-image.interface'; +import { MockProvider } from 'ng-mocks'; +import { LoadImageService } from 'app/shared/image-cropper/services/load-image.service'; +import { MoveStart } from 'app/shared/image-cropper/interfaces/move-start.interface'; +import { CropperPosition } from 'app/shared/image-cropper/interfaces/cropper-position.interface'; +import { CropperSettings } from 'app/shared/image-cropper/interfaces/cropper.settings'; +import { ElementRef } from '@angular/core'; +import { ImageCroppedEvent } from 'app/shared/image-cropper/interfaces/image-cropped-event.interface'; +describe('ImageCropperComponent', () => { + let fixture: ComponentFixture; + let comp: ImageCropperComponent; + let cropperPositionService: CropperPositionService; + let cropService: CropService; + let loadImageService: LoadImageService; + let resetCropperPositionSpy: jest.SpyInstance; + let componentCropSpy: jest.SpyInstance; + let startCropImageSpy: jest.SpyInstance; + let imageCroppedSpy: jest.SpyInstance; + let cropServiceCropSpy: jest.SpyInstance; + let loadImageFileSpy: jest.SpyInstance; + let loadBase64ImageSpy: jest.SpyInstance; + let loadImageFromURLSpy: jest.SpyInstance; + beforeEach(() => { + TestBed.configureTestingModule({ + imports: [], + declarations: [ImageCropperComponent], + providers: [MockProvider(CropService), MockProvider(CropperPositionService), MockProvider(LoadImageService)], + schemas: [], + }) + .compileComponents() + .then(() => { + fixture = TestBed.createComponent(ImageCropperComponent); + comp = fixture.componentInstance; + cropperPositionService = TestBed.inject(CropperPositionService); + cropService = TestBed.inject(CropService); + loadImageService = TestBed.inject(LoadImageService); + resetCropperPositionSpy = jest.spyOn(cropperPositionService, 'resetCropperPosition'); + componentCropSpy = jest.spyOn(comp, 'crop'); + startCropImageSpy = jest.spyOn(comp.startCropImage, 'emit'); + imageCroppedSpy = jest.spyOn(comp.imageCropped, 'emit'); + cropServiceCropSpy = jest.spyOn(cropService, 'crop'); + loadImageFileSpy = jest.spyOn(loadImageService, 'loadImageFile'); + loadBase64ImageSpy = jest.spyOn(loadImageService, 'loadBase64Image'); + loadImageFromURLSpy = jest.spyOn(loadImageService, 'loadImageFromURL'); + }); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + describe('reset cropper position', () => { + let cropper: CropperPosition; + let settings: CropperSettings; + let sourceImage: ElementRef; + + beforeEach(() => { + cropper = comp.cropper; + settings = comp.settings; + sourceImage = comp.sourceImage; + comp.imageVisible = false; + }); + + it('should reset cropper position without auto crop', () => { + comp.autoCrop = false; + + comp.resetCropperPosition(); + + expect(resetCropperPositionSpy).toHaveBeenCalledOnce(); + expect(resetCropperPositionSpy).toHaveBeenCalledWith(sourceImage, cropper, settings); + expect(componentCropSpy).toHaveBeenCalledTimes(0); + expect(comp.imageVisible).toBeTrue(); + }); + + it('should reset cropper position with auto crop', () => { + comp.autoCrop = true; + + comp.resetCropperPosition(); + + expect(resetCropperPositionSpy).toHaveBeenCalledOnce(); + expect(resetCropperPositionSpy).toHaveBeenCalledWith(sourceImage, cropper, settings); + expect(componentCropSpy).toHaveBeenCalledOnce(); + expect(comp.imageVisible).toBeTrue(); + }); + }); + + it('should not crop without loaded image', () => { + comp.loadedImage = undefined; + let res = comp.crop(); + expect(res).toBeUndefined(); + + comp.loadedImage = { transformed: {} } as LoadedImage; + res = comp.crop(); + + expect(res).toBeUndefined(); + }); + + describe('crop', () => { + let cropper: CropperPosition; + let settings: CropperSettings; + let sourceImage: ElementRef; + let loadedImage: LoadedImage; + let fakeEvent: ImageCroppedEvent; + + beforeEach(() => { + cropper = comp.cropper; + settings = comp.settings; + sourceImage = comp.sourceImage; + loadedImage = { transformed: { base64: 'base64', image: new Image(), size: { width: 100, height: 100 } } } as LoadedImage; + fakeEvent = { base64: 'base64', width: 100, height: 100, cropperPosition: { x1: 42, y1: 42, x2: 42, y2: 42 }, imagePosition: { x1: 42, y1: 42, x2: 42, y2: 42 } }; + }); + + it('should crop and emit with loaded image', () => { + cropServiceCropSpy.mockReturnValue(fakeEvent); + comp.loadedImage = loadedImage; + + const res = comp.crop(); + + expect(startCropImageSpy).toHaveBeenCalledOnce(); + expect(startCropImageSpy).toHaveBeenCalledWith(); + expect(cropServiceCropSpy).toHaveBeenCalledOnce(); + expect(cropServiceCropSpy).toHaveBeenCalledWith(sourceImage, loadedImage, cropper, settings); + expect(imageCroppedSpy).toHaveBeenCalledOnce(); + expect(imageCroppedSpy).toHaveBeenCalledWith(fakeEvent); + expect(res).toBe(fakeEvent); + }); + + it('should crop but not emit without cropping output', () => { + cropServiceCropSpy.mockReturnValue(undefined); + comp.loadedImage = loadedImage; + + const res = comp.crop(); + + expect(startCropImageSpy).toHaveBeenCalledOnce(); + expect(startCropImageSpy).toHaveBeenCalledWith(); + expect(cropServiceCropSpy).toHaveBeenCalledOnce(); + expect(cropServiceCropSpy).toHaveBeenCalledWith(sourceImage, loadedImage, cropper, settings); + expect(imageCroppedSpy).toHaveBeenCalledTimes(0); + expect(res).toBeUndefined(); + }); + }); + + it('should reset when removing image', () => { + loadImageFileSpy.mockImplementation(() => Promise.resolve({ transformed: {} } as LoadedImage)); + fixture.componentRef.setInput('imageFile', new File([], 'test')); + fixture.detectChanges(); + loadImageFileSpy.mockClear(); + + comp.imageVisible = true; + comp.loadedImage = { transformed: {} } as LoadedImage; + comp.cropper = { x1: 42, y1: 42, x2: 42, y2: 42 }; + comp.maxSize = { width: 42, height: 42 }; + comp.moveStart = {} as MoveStart; + + fixture.componentRef.setInput('imageFile', undefined); + fixture.detectChanges(); + + expect(comp.imageVisible).toBeFalse(); + expect(comp.loadedImage).toBeUndefined(); + expect(comp.cropper).toEqual({ x1: -100, y1: -100, x2: 10000, y2: 10000 }); + expect(comp.maxSize).toEqual({ width: 0, height: 0 }); + expect(comp.moveStart).not.toEqual({}); + }); + + it('should not reset when image does not get changed', () => { + loadImageFileSpy.mockImplementation(() => Promise.resolve({ transformed: {} } as LoadedImage)); + fixture.componentRef.setInput('imageFile', new File([], 'test')); + fixture.detectChanges(); + loadImageFileSpy.mockClear(); + + const loadedImage = { transformed: {}, original: { image: { complete: true } } } as LoadedImage; + const cropper = { x1: 42, y1: 42, x2: 42, y2: 42 }; + const maxSize = { width: 42, height: 42 }; + const moveStart = {} as MoveStart; + comp.imageVisible = true; + comp.loadedImage = loadedImage; + comp.cropper = cropper; + comp.maxSize = maxSize; + comp.moveStart = moveStart; + + fixture.componentRef.setInput('autoCrop', false); + fixture.detectChanges(); + + expect(comp.imageVisible).toBeTrue(); + expect(comp.loadedImage).toBe(loadedImage); + expect(comp.cropper).toBe(cropper); + expect(comp.maxSize).toBe(maxSize); + expect(comp.moveStart).toBe(moveStart); + }); + + describe('load image', () => { + const base64String = Buffer.from('testContent').toString('base64'); + + it('should load from file', () => { + loadImageFileSpy.mockImplementation(() => Promise.resolve({ transformed: { base64: base64String } } as LoadedImage)); + const file = new File([], 'test'); + const settings = comp.settings; + fixture.componentRef.setInput('imageFile', file); + fixture.detectChanges(); + + expect(loadImageFileSpy).toHaveBeenCalledOnce(); + expect(loadImageFileSpy).toHaveBeenCalledWith(file, settings); + expect(loadImageFromURLSpy).toHaveBeenCalledTimes(0); + expect(loadBase64ImageSpy).toHaveBeenCalledTimes(0); + }); + + it('should load from URL', () => { + loadImageFromURLSpy.mockImplementation(() => Promise.resolve({ transformed: { base64: base64String } } as LoadedImage)); + const url = 'https://test.com/path_to_image.png'; + const settings = comp.settings; + fixture.componentRef.setInput('imageURL', url); + fixture.detectChanges(); + + expect(loadImageFromURLSpy).toHaveBeenCalledOnce(); + expect(loadImageFromURLSpy).toHaveBeenCalledWith(url, settings); + expect(loadImageFileSpy).toHaveBeenCalledTimes(0); + expect(loadBase64ImageSpy).toHaveBeenCalledTimes(0); + }); + + it('should load from Base64', () => { + loadBase64ImageSpy.mockImplementation(() => Promise.resolve({ transformed: { base64: base64String } } as LoadedImage)); + const content = base64String; + const settings = comp.settings; + fixture.componentRef.setInput('imageBase64', content); + fixture.detectChanges(); + + expect(loadBase64ImageSpy).toHaveBeenCalledOnce(); + expect(loadBase64ImageSpy).toHaveBeenCalledWith(content, settings); + expect(loadImageFileSpy).toHaveBeenCalledTimes(0); + expect(loadImageFromURLSpy).toHaveBeenCalledTimes(0); + }); + + it('should load from event', () => { + loadImageFileSpy.mockImplementation(() => Promise.resolve({ transformed: { base64: base64String } } as LoadedImage)); + const file = new File([], 'test'); + const event = { currentTarget: { files: [file] } as unknown as HTMLInputElement }; + const settings = comp.settings; + fixture.componentRef.setInput('imageChangedEvent', event); + fixture.detectChanges(); + + expect(loadImageFileSpy).toHaveBeenCalledOnce(); + expect(loadImageFileSpy).toHaveBeenCalledWith(file, settings); + }); + }); +}); From 071c53b2e73e228f5821b540b2d12093e2816c28 Mon Sep 17 00:00:00 2001 From: Julian Christl Date: Thu, 27 Oct 2022 12:12:46 +0200 Subject: [PATCH 15/16] fix cypress tests --- src/test/cypress/integration/Logout.spec.ts | 3 ++- src/test/cypress/support/requests/CourseManagementRequests.ts | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/cypress/integration/Logout.spec.ts b/src/test/cypress/integration/Logout.spec.ts index 8736f4c166d6..ee51b1312b9c 100644 --- a/src/test/cypress/integration/Logout.spec.ts +++ b/src/test/cypress/integration/Logout.spec.ts @@ -2,6 +2,7 @@ import { artemis } from '../support/ArtemisTesting'; import { authTokenKey } from '../support/constants'; import { Course } from '../../../main/webapp/app/entities/course.model'; import { ModelingExercise } from '../../../main/webapp/app/entities/modeling-exercise.model'; +import { convertCourseAfterMultiPart } from '../support/requests/CourseManagementRequests'; const courseRequests = artemis.requests.courseManagement; const users = artemis.users; @@ -18,7 +19,7 @@ describe('Logout tests', () => { cy.login(admin); courseRequests.createCourse(true).then((response) => { - course = response.body; + course = convertCourseAfterMultiPart(response); courseRequests.createModelingExercise({ course }).then((resp: Cypress.Response) => { modelingExercise = resp.body; }); diff --git a/src/test/cypress/support/requests/CourseManagementRequests.ts b/src/test/cypress/support/requests/CourseManagementRequests.ts index 928aee6ae959..f62ef9fcf4d2 100644 --- a/src/test/cypress/support/requests/CourseManagementRequests.ts +++ b/src/test/cypress/support/requests/CourseManagementRequests.ts @@ -75,7 +75,6 @@ export class CourseManagementRequests { course.instructorGroupName = Cypress.env('instructorGroupName'); } const formData = new FormData(); - Cypress.Blob; formData.append('course', new File([JSON.stringify(course)], 'course', { type: 'application/json' })); return cy.request({ url: BASE_API + 'courses', From ec4e7810cdf451292a62e58818009b8fb47e09ce Mon Sep 17 00:00:00 2001 From: Julian Christl Date: Tue, 1 Nov 2022 23:43:22 +0100 Subject: [PATCH 16/16] fix linting --- .../exercises/programming/ProgrammingExerciseManagement.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/cypress/integration/exercises/programming/ProgrammingExerciseManagement.spec.ts b/src/test/cypress/integration/exercises/programming/ProgrammingExerciseManagement.spec.ts index 02954ce353c6..615e43f04ac6 100644 --- a/src/test/cypress/integration/exercises/programming/ProgrammingExerciseManagement.spec.ts +++ b/src/test/cypress/integration/exercises/programming/ProgrammingExerciseManagement.spec.ts @@ -4,7 +4,7 @@ import { Course } from 'app/entities/course.model'; import { DELETE } from '../../../support/constants'; import { artemis } from '../../../support/ArtemisTesting'; import { generateUUID } from '../../../support/utils'; -import { convertCourseAfterMultiPart, PROGRAMMING_EXERCISE_BASE } from '../../../support/requests/CourseManagementRequests'; +import { PROGRAMMING_EXERCISE_BASE, convertCourseAfterMultiPart } from '../../../support/requests/CourseManagementRequests'; // Admin account const admin = artemis.users.getAdmin();