Development: Reduce transferred data when creating ratings (#7168)

src/main/java/de/tum/in/www1/artemis/web/rest/ComplaintResource.java

@@ -102,7 +102,7 @@ public ResponseEntity<Complaint> createComplaint(@RequestBody Complaint complain
         Complaint savedComplaint = complaintService.createComplaint(complaint, OptionalLong.empty(), principal);
 
         // Remove assessor information from client request
-        savedComplaint.getResult().setAssessor(null);
+        savedComplaint.getResult().filterSensitiveInformation();
 
         return ResponseEntity.created(new URI("/api/complaints/" + savedComplaint.getId()))
                 .headers(HeaderUtil.createEntityCreationAlert(applicationName, true, entityName, savedComplaint.getId().toString())).body(savedComplaint);
@@ -138,7 +138,7 @@ public ResponseEntity<Complaint> createComplaintForExamExercise(@PathVariable Lo
         Complaint savedComplaint = complaintService.createComplaint(complaint, OptionalLong.of(examId), principal);
 
         // Remove assessor information from client request
-        savedComplaint.getResult().setAssessor(null);
+        savedComplaint.getResult().filterSensitiveInformation();
 
         return ResponseEntity.created(new URI("/api/complaints/" + savedComplaint.getId()))
                 .headers(HeaderUtil.createEntityCreationAlert(applicationName, true, entityName, savedComplaint.getId().toString())).body(savedComplaint);

src/main/java/de/tum/in/www1/artemis/web/rest/RatingResource.java

@@ -11,10 +11,7 @@
 import org.springframework.validation.annotation.Validated;
 import org.springframework.web.bind.annotation.*;
 
-import de.tum.in.www1.artemis.domain.Course;
-import de.tum.in.www1.artemis.domain.Rating;
-import de.tum.in.www1.artemis.domain.Result;
-import de.tum.in.www1.artemis.domain.User;
+import de.tum.in.www1.artemis.domain.*;
 import de.tum.in.www1.artemis.domain.participation.StudentParticipation;
 import de.tum.in.www1.artemis.repository.CourseRepository;
 import de.tum.in.www1.artemis.repository.ResultRepository;
@@ -59,37 +56,37 @@ public RatingResource(RatingService ratingService, UserRepository userRepository
     }
 
     /**
-     * Return Rating referencing resultId or null
+     * GET /results/:resultId/rating : Return Rating referencing resultId or null
      *
      * @param resultId - Id of result that is referenced with the rating
-     * @return Rating or null
+     * @return saved star rating value or empty optional
      */
     @GetMapping("/results/{resultId}/rating")
     @EnforceAtLeastStudent
-    public ResponseEntity<Optional<Rating>> getRatingForResult(@PathVariable Long resultId) {
+    public ResponseEntity<Optional<Integer>> getRatingForResult(@PathVariable Long resultId) {
         // TODO allow for Instructors
         if (!authCheckService.isAdmin()) {
             checkIfUserIsOwnerOfSubmissionElseThrow(resultId);
         }
         Optional<Rating> rating = ratingService.findRatingByResultId(resultId);
-        return ResponseEntity.ok(rating);
+        return ResponseEntity.ok(rating.map(Rating::getRating));
     }
 
     /**
-     * Persist a new Rating
+     * POST /results/:resultId/rating/:ratingValue : Persist a new Rating
      *
      * @param resultId    - Id of result that is referenced with the rating that should be persisted
      * @param ratingValue - Value of the updated rating
-     * @return inserted Rating
+     * @return inserted star rating value
      * @throws URISyntaxException if the Location URI syntax is incorrect
      */
     @PostMapping("/results/{resultId}/rating/{ratingValue}")
     @EnforceAtLeastStudent
-    public ResponseEntity<Rating> createRatingForResult(@PathVariable long resultId, @PathVariable int ratingValue) throws URISyntaxException {
+    public ResponseEntity<Integer> createRatingForResult(@PathVariable long resultId, @PathVariable int ratingValue) throws URISyntaxException {
         checkRating(ratingValue);
         checkIfUserIsOwnerOfSubmissionElseThrow(resultId);
         Rating savedRating = ratingService.saveRating(resultId, ratingValue);
-        return ResponseEntity.created(new URI("/api/results/" + savedRating.getId() + "/rating")).body(savedRating);
+        return ResponseEntity.created(new URI("/api/results/" + savedRating.getId() + "/rating")).body(savedRating.getRating());
     }
 
     private void checkRating(int ratingValue) {
@@ -99,23 +96,23 @@ private void checkRating(int ratingValue) {
     }
 
     /**
-     * Update a Rating
+     * PUT /results/:resultId/rating/:ratingValue : Update a Rating
      *
      * @param resultId    - Id of result that is referenced with the rating that should be updated
      * @param ratingValue - Value of the updated rating
-     * @return updated Rating
+     * @return updated star rating value
      */
     @PutMapping("/results/{resultId}/rating/{ratingValue}")
     @EnforceAtLeastStudent
-    public ResponseEntity<Rating> updateRatingForResult(@PathVariable long resultId, @PathVariable int ratingValue) {
+    public ResponseEntity<Integer> updateRatingForResult(@PathVariable long resultId, @PathVariable int ratingValue) {
         checkRating(ratingValue);
         checkIfUserIsOwnerOfSubmissionElseThrow(resultId);
         Rating savedRating = ratingService.updateRating(resultId, ratingValue);
-        return ResponseEntity.ok(savedRating);
+        return ResponseEntity.ok(savedRating.getRating());
     }
 
     /**
-     * Get all ratings for the "courseId" Course
+     * GET /course/:courseId/rating : Get all ratings for the "courseId" Course
      *
      * @param courseId - Id of the course that the ratings are fetched for
      * @return List of Ratings for the course
@@ -126,6 +123,11 @@ public ResponseEntity<List<Rating>> getRatingForInstructorDashboard(@PathVariabl
         Course course = courseRepository.findByIdElseThrow(courseId);
         authCheckService.checkHasAtLeastRoleInCourseElseThrow(Role.INSTRUCTOR, course, null);
         List<Rating> responseRatings = ratingService.getAllRatingsByCourse(courseId);
+        responseRatings.forEach(rating -> {
+            rating.getResult().setSubmission(null);
+            rating.getResult().getParticipation().getExercise().setCourse(null);
+            rating.getResult().getParticipation().getExercise().setExerciseGroup(null);
+        });
         return ResponseEntity.ok(responseRatings);
     }
 

src/main/webapp/app/exam/manage/exams/exam-exercise-import/exam-exercise-import.component.ts

@@ -1,10 +1,9 @@
 import { Component, Input, OnInit } from '@angular/core';
 import { Exam } from 'app/entities/exam.model';
 import { faCheckDouble, faFont } from '@fortawesome/free-solid-svg-icons';
-import { Exercise, ExerciseType } from 'app/entities/exercise.model';
+import { Exercise, ExerciseType, getIcon } from 'app/entities/exercise.model';
 import { ExerciseGroup } from 'app/entities/exercise-group.model';
 import { SHORT_NAME_PATTERN } from 'app/shared/constants/input.constants';
-import { getIcon } from 'app/entities/exercise.model';
 
 @Component({
     selector: 'jhi-exam-exercise-import',

src/main/webapp/app/exercises/shared/rating/rating.component.html

@@ -1,6 +1,6 @@
-<div [class.non-clickable]="disableRating" *ngIf="rating">
+<div [class.non-clickable]="disableRating">
     <b>
         <span jhiTranslate="artemisApp.rating.label"></span>
     </b>
-    <star-rating checkedColor="gold" uncheckedColor="grey" [value]="rating?.rating || 0" [size]="'24'" [readOnly]="false" [totalStars]="5" (rate)="onRate($event)"> </star-rating>
+    <star-rating checkedColor="gold" uncheckedColor="grey" [value]="rating || 0" [size]="'24'" [readOnly]="false" [totalStars]="5" (rate)="onRate($event)"> </star-rating>
 </div>

src/main/webapp/app/exercises/shared/rating/rating.component.ts

@@ -2,17 +2,17 @@ import { Component, Input, OnInit } from '@angular/core';
 import { RatingService } from 'app/exercises/shared/rating/rating.service';
 import { StarRatingComponent } from 'app/exercises/shared/rating/star-rating/star-rating.component';
 import { Result } from 'app/entities/result.model';
-import { Rating } from 'app/entities/rating.model';
 import { StudentParticipation } from 'app/entities/participation/student-participation.model';
 import { AccountService } from 'app/core/auth/account.service';
+import { Observable } from 'rxjs';
 
 @Component({
     selector: 'jhi-rating',
     templateUrl: './rating.component.html',
     styleUrls: ['./rating.component.scss'],
 })
 export class RatingComponent implements OnInit {
-    public rating: Rating;
+    public rating: number;
     public disableRating = false;
     @Input() result?: Result;
 
@@ -22,16 +22,12 @@ export class RatingComponent implements OnInit {
     ) {}
 
     ngOnInit(): void {
-        if (!this.result || !this.result.id || !this.result.participation || !this.accountService.isOwnerOfParticipation(this.result.participation as StudentParticipation)) {
+        if (!this.result?.id || !this.result.participation || !this.accountService.isOwnerOfParticipation(this.result.participation as StudentParticipation)) {
             return;
         }
 
         this.ratingService.getRating(this.result.id).subscribe((rating) => {
-            if (rating) {
-                this.rating = rating;
-            } else {
-                this.rating = new Rating(this.result, 0);
-            }
+            this.rating = rating ?? 0;
         });
     }
 
@@ -41,24 +37,22 @@ export class RatingComponent implements OnInit {
      */
     onRate(event: { oldValue: number; newValue: number; starRating: StarRatingComponent }) {
         // block rating to prevent double sending of post request
-        if (this.disableRating || !this.rating.result) {
+        if (this.disableRating || !this.result) {
             return;
         }
 
-        // update feedback locally
-        this.rating.rating = event.newValue;
+        const oldRating = this.rating;
+        this.rating = event.newValue;
 
+        this.disableRating = true;
+        let observable: Observable<number>;
         // set/update feedback on the server
-        if (this.rating.id) {
-            this.ratingService.updateRating(this.rating).subscribe((rating) => {
-                this.rating = rating;
-            });
+        if (oldRating) {
+            observable = this.ratingService.updateRating(this.rating, this.result.id!);
         } else {
-            this.disableRating = true;
-            this.ratingService.createRating(this.rating).subscribe((rating) => {
-                this.rating = rating;
-                this.disableRating = false;
-            });
+            observable = this.ratingService.createRating(this.rating, this.result.id!);
         }
+
+        observable.subscribe((rating) => (this.rating = rating)).add(() => (this.disableRating = false));
     }
 }

src/main/webapp/app/exercises/shared/rating/rating.service.ts

@@ -13,26 +13,28 @@ export class RatingService {
 
     /**
      * Create the student rating for feedback on the server.
-     * @param rating - Rating for the result
+     * @param rating - star rating for the result
+     * @param resultId - id of the linked result
      */
-    createRating(rating: Rating): Observable<Rating> {
-        return this.http.post<Rating>(this.ratingResourceUrl + `${rating.result!.id!}/rating/${rating.rating}`, null);
+    createRating(rating: number, resultId: number): Observable<number> {
+        return this.http.post<number>(this.ratingResourceUrl + `${resultId}/rating/${rating}`, null);
     }
 
     /**
      * Get rating for "resultId" Result
-     * @param ratingId - Id of Result who's rating is received
+     * @param resultId - id of result who's rating is received
      */
-    getRating(ratingId: number): Observable<Rating | null> {
-        return this.http.get<Rating | null>(this.ratingResourceUrl + `${ratingId}/rating`);
+    getRating(resultId: number): Observable<number | null> {
+        return this.http.get<number | null>(this.ratingResourceUrl + `${resultId}/rating`);
     }
 
     /**
      * Update rating for "resultId" Result
-     * @param rating - Rating for the result
+     * @param rating - star rating for the result
+     * @param resultId - id of the linked result
      */
-    updateRating(rating: Rating): Observable<Rating> {
-        return this.http.put<Rating>(this.ratingResourceUrl + `${rating.result!.id!}/rating/${rating.rating}`, null);
+    updateRating(rating: number, resultId: number): Observable<number> {
+        return this.http.put<number>(this.ratingResourceUrl + `${resultId}/rating/${rating}`, null);
     }
 
     /**

src/test/java/de/tum/in/www1/artemis/assessment/RatingResourceIntegrationTest.java

@@ -4,9 +4,10 @@
 
 import java.util.Optional;
 
-import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.http.HttpStatus;
 import org.springframework.security.test.context.support.WithMockUser;
@@ -18,7 +19,6 @@
 import de.tum.in.www1.artemis.exercise.textexercise.TextExerciseUtilService;
 import de.tum.in.www1.artemis.participation.ParticipationFactory;
 import de.tum.in.www1.artemis.participation.ParticipationUtilService;
-import de.tum.in.www1.artemis.repository.RatingRepository;
 import de.tum.in.www1.artemis.service.RatingService;
 import de.tum.in.www1.artemis.user.UserUtilService;
 
@@ -29,9 +29,6 @@ class RatingResourceIntegrationTest extends AbstractSpringIntegrationIndependent
     @Autowired
     private RatingService ratingService;
 
-    @Autowired
-    private RatingRepository ratingRepo;
-
     @Autowired
     private UserUtilService userUtilService;
 
@@ -70,24 +67,21 @@ void initTestCase() {
         userUtilService.createAndSaveUser(TEST_PREFIX + "instructor2");
     }
 
-    @AfterEach
-    void tearDown() {
-        ratingRepo.deleteAllInBatch();
-    }
-
     @Test
     @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER")
     void testCreateRating_asUser() throws Exception {
-        request.post("/api/results/" + result.getId() + "/rating/" + rating.getRating(), null, HttpStatus.CREATED);
+        int response = request.postWithResponseBody("/api/results/" + result.getId() + "/rating/" + rating.getRating(), null, Integer.class, HttpStatus.CREATED);
         Rating savedRating = ratingService.findRatingByResultId(result.getId()).orElseThrow();
         assertThat(savedRating.getRating()).isEqualTo(2);
+        assertThat(response).isEqualTo(2);
         assertThat(savedRating.getResult().getId()).isEqualTo(result.getId());
     }
 
-    @Test
+    @ParameterizedTest(name = "{displayName} [{index}] {argumentsWithNames}")
+    @ValueSource(ints = { 7, 123, -5, 0 })
     @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER")
-    void testCreateInvalidRating_asUser() throws Exception {
-        rating.setRating(7);
+    void testCreateInvalidRating_asUser(int value) throws Exception {
+        rating.setRating(value);
         request.post("/api/results/" + result.getId() + "/rating/" + rating.getRating(), null, HttpStatus.BAD_REQUEST);
         final Optional<Rating> optionalRating = ratingService.findRatingByResultId(result.getId());
         assertThat(optionalRating).isEmpty();
@@ -103,20 +97,21 @@ void testCreateRating_asTutor_FORBIDDEN() throws Exception {
     @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER")
     void testGetRating_asUser() throws Exception {
         Rating savedRating = ratingService.saveRating(result.getId(), rating.getRating());
-        request.get("/api/results/" + savedRating.getResult().getId() + "/rating", HttpStatus.OK, Rating.class);
+        int response = request.get("/api/results/" + savedRating.getResult().getId() + "/rating", HttpStatus.OK, Integer.class);
+        assertThat(response).isEqualTo(2);
     }
 
     @Test
     @WithMockUser(username = TEST_PREFIX + "tutor1", roles = "TA")
     void testGetRating_asUser_FORBIDDEN() throws Exception {
         Rating savedRating = ratingService.saveRating(result.getId(), rating.getRating());
-        request.get("/api/results/" + savedRating.getResult().getId() + "/rating", HttpStatus.FORBIDDEN, Rating.class);
+        request.get("/api/results/" + savedRating.getResult().getId() + "/rating", HttpStatus.FORBIDDEN, Integer.class);
     }
 
     @Test
     @WithMockUser(username = TEST_PREFIX + "student1", roles = "USER")
     void testGetRating_asUser_Null() throws Exception {
-        Rating savedRating = request.get("/api/results/" + result.getId() + "/rating", HttpStatus.OK, Rating.class);
+        Integer savedRating = request.get("/api/results/" + result.getId() + "/rating", HttpStatus.OK, Integer.class);
         assertThat(savedRating).isNull();
     }
 

src/test/javascript/spec/component/rating/rating.component.spec.ts

@@ -6,7 +6,6 @@ import { RatingService } from 'app/exercises/shared/rating/rating.service';
 import { MockRatingService } from '../../helpers/mocks/service/mock-rating.service';
 import { Result } from 'app/entities/result.model';
 import { Submission } from 'app/entities/submission.model';
-import { Rating } from 'app/entities/rating.model';
 import { of } from 'rxjs';
 import { AccountService } from 'app/core/auth/account.service';
 import { MockAccountService } from '../../helpers/mocks/service/mock-account.service';
@@ -62,20 +61,19 @@ describe('RatingComponent', () => {
 
     it('should create new local rating', () => {
         ratingComponent.ngOnInit();
-        expect(ratingComponent.rating.result?.id).toBe(89);
-        expect(ratingComponent.rating.rating).toBe(0);
+        expect(ratingComponent.rating).toBe(0);
     });
 
     it('should set rating received from server', () => {
-        jest.spyOn(ratingService, 'getRating').mockReturnValue(of(new Rating({ id: 90 } as Result, 1)));
+        jest.spyOn(ratingService, 'getRating').mockReturnValue(of(1));
         ratingComponent.ngOnInit();
-        expect(ratingComponent.rating.result?.id).toBe(90);
-        expect(ratingComponent.rating.rating).toBe(1);
+        expect(ratingComponent.rating).toBe(1);
     });
 
     describe('OnRate', () => {
         beforeEach(() => {
-            ratingComponent.rating = new Rating({ id: 89 } as Result, 0);
+            ratingComponent.rating = 0;
+            ratingComponent.result = { id: 89 } as Result;
             jest.spyOn(ratingService, 'createRating');
             jest.spyOn(ratingService, 'updateRating');
         });
@@ -99,21 +97,19 @@ describe('RatingComponent', () => {
             });
             expect(ratingService.createRating).toHaveBeenCalledOnce();
             expect(ratingService.updateRating).not.toHaveBeenCalled();
-            expect(ratingComponent.rating.result?.id).toBe(89);
-            expect(ratingComponent.rating.rating).toBe(2);
+            expect(ratingComponent.rating).toBe(2);
         });
 
         it('should update rating', () => {
-            ratingComponent.rating.id = 89;
+            ratingComponent.rating = 1;
             ratingComponent.onRate({
-                oldValue: 0,
+                oldValue: 1,
                 newValue: 2,
                 starRating: new StarRatingComponent(),
             });
             expect(ratingService.updateRating).toHaveBeenCalledOnce();
             expect(ratingService.createRating).not.toHaveBeenCalled();
-            expect(ratingComponent.rating.result?.id).toBe(89);
-            expect(ratingComponent.rating.rating).toBe(2);
+            expect(ratingComponent.rating).toBe(2);
         });
     });
 });