Development: Match comment ratings to their author by id

The comment section paired each comment with a rating by comparing
display names, so two reviewers called "Max Mustermann" would show each
other's score. Names are not unique and never were, and the client had
nothing else to match on: neither the comment nor the rating carried the
id of the user behind it.

Both DTOs now expose one, named after the flat variants that already do
this: RatingDTO gains fromUserId and InternalCommentDTO gains
authorUserId. The client keys the lookup on those instead.

This also makes the ratings distinct on the server. RatingOverviewDTO
holds a Set, so before this two reviewers sharing a name who happened to
give the same score collapsed into a single entry.

Co-Authored-By: Claude <noreply@anthropic.com>

Author: az108

openapi/openapi.yaml

@@ -3544,6 +3544,7 @@ components:
       type: object
       properties:
         author: {type: string}
+        authorUserId: {type: string, format: uuid}
         canEdit: {type: boolean}
         commentId: {type: string, format: uuid}
         createdAt: {type: string, format: date-time}
@@ -4071,6 +4072,7 @@ components:
       type: object
       properties:
         from: {type: string}
+        fromUserId: {type: string, format: uuid}
         rating: {type: integer, format: int32}
     RatingOverviewDTO:
       type: object

src/main/java/de/tum/cit/aet/evaluation/dto/InternalCommentDTO.java

@@ -7,7 +7,7 @@
 import java.util.UUID;
 import lombok.NonNull;
 
-public record InternalCommentDTO(UUID commentId, String author, String message, Instant createdAt, boolean canEdit) {
+public record InternalCommentDTO(UUID commentId, UUID authorUserId, String author, String message, Instant createdAt, boolean canEdit) {
     /**
      * Creates a DTO representation of an internal comment for the given user context.
      *
@@ -22,6 +22,7 @@ public static InternalCommentDTO from(@NonNull InternalComment comment, @NonNull
         User author = comment.getCreatedBy();
         return new InternalCommentDTO(
             comment.getInternalCommentId(),
+            author.getUserId(),
             author.getFirstName() + " " + author.getLastName(),
             comment.getMessage(),
             comment.getCreatedAt().toInstant(ZoneOffset.UTC),

src/main/java/de/tum/cit/aet/evaluation/dto/RatingDTO.java

@@ -1,15 +1,18 @@
 package de.tum.cit.aet.evaluation.dto;
 
 import de.tum.cit.aet.evaluation.domain.Rating;
+import de.tum.cit.aet.usermanagement.domain.User;
+import java.util.UUID;
 
-public record RatingDTO(String from, int rating) {
+public record RatingDTO(UUID fromUserId, String from, int rating) {
     /**
      * Creates a {@link RatingDTO} from a given {@link Rating} entity.
      *
      * @param rating the {@link Rating} entity to convert; must not be {@code null}
-     * @return a {@link RatingDTO} containing the rater's full name and rating value
+     * @return a {@link RatingDTO} containing the rater's id, full name and rating value
      */
     public static RatingDTO from(Rating rating) {
-        return new RatingDTO(rating.getFrom().getFirstName() + " " + rating.getFrom().getLastName(), rating.getRating());
+        User rater = rating.getFrom();
+        return new RatingDTO(rater.getUserId(), rater.getFirstName() + " " + rater.getLastName(), rating.getRating());
     }
 }

src/main/webapp/app/generated/model/internal-comment-dto.ts

@@ -11,6 +11,7 @@
 
 export interface InternalCommentDTO {
     readonly author?: string;
+    readonly authorUserId?: string;
     readonly canEdit?: boolean;
     readonly commentId?: string;
     readonly createdAt?: string;

src/main/webapp/app/generated/model/rating-dto.ts

@@ -11,5 +11,6 @@
 
 export interface RatingDTO {
     readonly from?: string;
+    readonly fromUserId?: string;
     readonly rating?: number;
 }

src/main/webapp/app/shared/components/molecules/comment-section/comment-section.html

@@ -12,7 +12,7 @@
           [canEdit]="c.canEdit ?? false"
           [isCreate]="false"
           [text]="c.message ?? ''"
-          [rating]="ratingByAuthor().get(c.author ?? '')"
+          [rating]="ratingByAuthorId().get(c.authorUserId ?? '')"
           [editingId]="editingId()"
           (enterEdit)="editingId.set(c.commentId)"
           (exitEdit)="editingId.set(undefined)"

src/main/webapp/app/shared/components/molecules/comment-section/comment-section.ts

@@ -26,6 +26,7 @@ export class CommentSection {
   protected comments = signal<InternalCommentDTO[]>([]);
   protected createDraft = signal<string>('');
   protected currentUser = this.accountService.loadedUser()?.name ?? '';
+  protected currentUserId = this.accountService.loadedUser()?.id ?? '';
   protected editingId = signal<string | undefined>(undefined);
 
   protected readonly _loadCommentsEffect = effect(() => {
@@ -38,20 +39,21 @@ export class CommentSection {
     }
   });
 
-  protected readonly ratingByAuthor = computed<Map<string, number>>(() => {
+  /** Ratings keyed by the id of the reviewer who gave them, since display names are not unique. */
+  protected readonly ratingByAuthorId = computed<Map<string, number>>(() => {
     const map = new Map<string, number>();
     const overview = this.ratings();
     if (overview === undefined) {
       return map;
     }
 
     const currentRating = overview.currentUserRating;
-    if (this.currentUser !== '' && currentRating !== undefined) {
-      map.set(this.currentUser, currentRating);
+    if (this.currentUserId !== '' && currentRating !== undefined) {
+      map.set(this.currentUserId, currentRating);
     }
     for (const r of overview.otherRatings ?? []) {
-      if (r.from !== undefined && r.rating !== undefined) {
-        map.set(r.from, r.rating);
+      if (r.fromUserId !== undefined && r.rating !== undefined) {
+        map.set(r.fromUserId, r.rating);
       }
     }
     return map;

src/test/java/de/tum/cit/aet/evaluation/web/rest/InternalCommentResourceTest.java

@@ -116,6 +116,23 @@ void listCommentsReturnsOrderedList() {
             assertThat(list.get(2).message()).isEqualTo("second");
         }
 
+        @Test
+        void shouldIdentifyTheAuthorByIdWhenListingComments() {
+            internalCommentRepository.save(InternalCommentTestData.newCommentAll(application, otherProfessor, "from the other professor"));
+
+            List<InternalCommentDTO> list = api
+                .with(JwtPostProcessors.jwtUser(professor.getUserId(), "ROLE_PROFESSOR"))
+                .getAndRead(applicationCommentsUrl(), Map.of(), new TypeReference<>() {}, 200);
+
+            assertThat(list)
+                .filteredOn(c -> "from the other professor".equals(c.message()))
+                .singleElement()
+                .satisfies(c -> {
+                    assertThat(c.authorUserId()).isEqualTo(otherProfessor.getUserId());
+                    assertThat(c.canEdit()).isFalse();
+                });
+        }
+
         @Test
         void nonExistentApplicationReturns404() {
             UUID fakeAppId = UUID.randomUUID();

src/test/java/de/tum/cit/aet/evaluation/web/rest/RatingResourceTest.java

@@ -1,10 +1,12 @@
 package de.tum.cit.aet.evaluation.web.rest;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.tuple;
 
 import de.tum.cit.aet.AbstractResourceTest;
 import de.tum.cit.aet.application.domain.Application;
 import de.tum.cit.aet.application.repository.ApplicationRepository;
+import de.tum.cit.aet.evaluation.dto.RatingDTO;
 import de.tum.cit.aet.evaluation.dto.RatingOverviewDTO;
 import de.tum.cit.aet.evaluation.repository.RatingRepository;
 import de.tum.cit.aet.job.constants.JobState;
@@ -97,6 +99,25 @@ private String ratingsUrl(UUID applicationId) {
         return "/api/applications/" + applicationId + "/ratings";
     }
 
+    private User savedProfessorNamed(String email, String firstName, String lastName) {
+        return UserTestData.savedProfessorAll(
+            userRepository,
+            researchGroup,
+            null,
+            email,
+            firstName,
+            lastName,
+            "en",
+            null,
+            null,
+            null,
+            "DE",
+            null,
+            null,
+            UUID.randomUUID().toString().replace("-", "").substring(0, 7)
+        );
+    }
+
     @Nested
     class GetRatings {
 
@@ -124,6 +145,39 @@ void includesOtherProfessorRatings() {
             );
         }
 
+        @Test
+        void shouldIdentifyTheRaterByIdWhenReturningOtherRatings() {
+            RatingTestData.saved(ratingRepository, application, otherProfessor, -1);
+
+            RatingOverviewDTO overview = api
+                .with(JwtPostProcessors.jwtUser(professor.getUserId(), "ROLE_PROFESSOR"))
+                .getAndRead(ratingsUrl(), Map.of(), RatingOverviewDTO.class, 200);
+
+            assertThat(overview.otherRatings())
+                .singleElement()
+                .satisfies(r -> {
+                    assertThat(r.fromUserId()).isEqualTo(otherProfessor.getUserId());
+                    assertThat(r.rating()).isEqualTo(-1);
+                });
+        }
+
+        @Test
+        void shouldKeepRatingsApartWhenTwoRatersShareADisplayName() {
+            User firstNamesake = savedProfessorNamed("namesake.one@tum.de", "Max", "Mustermann");
+            User secondNamesake = savedProfessorNamed("namesake.two@tum.de", "Max", "Mustermann");
+            RatingTestData.saved(ratingRepository, application, firstNamesake, -2);
+            RatingTestData.saved(ratingRepository, application, secondNamesake, 2);
+
+            RatingOverviewDTO overview = api
+                .with(JwtPostProcessors.jwtUser(professor.getUserId(), "ROLE_PROFESSOR"))
+                .getAndRead(ratingsUrl(), Map.of(), RatingOverviewDTO.class, 200);
+
+            assertThat(overview.otherRatings()).hasSize(2);
+            assertThat(overview.otherRatings())
+                .extracting(RatingDTO::fromUserId, RatingDTO::rating)
+                .containsExactlyInAnyOrder(tuple(firstNamesake.getUserId(), -2), tuple(secondNamesake.getUserId(), 2));
+        }
+
         @Test
         void nonexistentApplicationReturns404() {
             UUID fakeAppId = UUID.randomUUID();

src/test/webapp/app/shared/components/molecules/comment-section/comment-section.spec.ts

@@ -240,7 +240,7 @@ describe('CommentSection', () => {
   });
 
   // ---------------- RATINGS ----------------
-  describe('ratingByAuthor', () => {
+  describe('ratingByAuthorId', () => {
     const setRatings = (ratings: RatingOverviewDTO | undefined): void => {
       fixture.componentRef.setInput('ratings', ratings);
       fixture.detectChanges();
@@ -254,59 +254,72 @@ describe('CommentSection', () => {
     it('should be empty when no ratings are provided', () => {
       setRatings(undefined);
 
-      expect(component['ratingByAuthor']().size).toBe(0);
+      expect(component['ratingByAuthorId']().size).toBe(0);
     });
 
     it('should map the current user to their own rating', () => {
       setRatings({ currentUserRating: 2, otherRatings: [] });
 
-      expect(component['ratingByAuthor']().get('Alice Reviewer')).toBe(2);
+      expect(component['ratingByAuthorId']().get('reviewer-1')).toBe(2);
     });
 
     it('should map every other reviewer to their rating', () => {
       setRatings({
         currentUserRating: 2,
         otherRatings: [
-          { from: 'Bob Reviewer', rating: -1 },
-          { from: 'Carol Reviewer', rating: 1 },
+          { fromUserId: 'reviewer-2', from: 'Bob Reviewer', rating: -1 },
+          { fromUserId: 'reviewer-3', from: 'Carol Reviewer', rating: 1 },
         ],
       });
 
-      const map = component['ratingByAuthor']();
-      expect(map.get('Bob Reviewer')).toBe(-1);
-      expect(map.get('Carol Reviewer')).toBe(1);
+      const map = component['ratingByAuthorId']();
+      expect(map.get('reviewer-2')).toBe(-1);
+      expect(map.get('reviewer-3')).toBe(1);
       expect(map.size).toBe(3);
     });
 
+    it('should keep reviewers who share a display name apart', () => {
+      setRatings({
+        otherRatings: [
+          { fromUserId: 'reviewer-2', from: 'Max Mustermann', rating: -2 },
+          { fromUserId: 'reviewer-3', from: 'Max Mustermann', rating: 2 },
+        ],
+      });
+
+      const map = component['ratingByAuthorId']();
+      expect(map.get('reviewer-2')).toBe(-2);
+      expect(map.get('reviewer-3')).toBe(2);
+    });
+
     it('should keep a rating of zero rather than dropping it', () => {
-      setRatings({ currentUserRating: 0, otherRatings: [{ from: 'Bob Reviewer', rating: 0 }] });
+      setRatings({ currentUserRating: 0, otherRatings: [{ fromUserId: 'reviewer-2', from: 'Bob Reviewer', rating: 0 }] });
 
-      const map = component['ratingByAuthor']();
-      expect(map.get('Alice Reviewer')).toBe(0);
-      expect(map.get('Bob Reviewer')).toBe(0);
+      const map = component['ratingByAuthorId']();
+      expect(map.get('reviewer-1')).toBe(0);
+      expect(map.get('reviewer-2')).toBe(0);
     });
 
-    it('should skip entries without an author or a rating', () => {
+    it('should skip entries without a reviewer id or a rating', () => {
       setRatings({
-        otherRatings: [{ from: 'Bob Reviewer' }, { rating: 1 }],
+        otherRatings: [{ from: 'Bob Reviewer', rating: 1 }, { fromUserId: 'reviewer-2' }],
       });
 
-      expect(component['ratingByAuthor']().size).toBe(0);
+      expect(component['ratingByAuthorId']().size).toBe(0);
     });
 
     it('should not map the current user when they have not rated', () => {
-      setRatings({ otherRatings: [{ from: 'Bob Reviewer', rating: 1 }] });
+      setRatings({ otherRatings: [{ fromUserId: 'reviewer-2', from: 'Bob Reviewer', rating: 1 }] });
 
-      const map = component['ratingByAuthor']();
-      expect(map.has('Alice Reviewer')).toBe(false);
+      const map = component['ratingByAuthorId']();
+      expect(map.has('reviewer-1')).toBe(false);
       expect(map.size).toBe(1);
     });
 
     it('should update when the ratings input changes', () => {
       setRatings({ currentUserRating: 1, otherRatings: [] });
       setRatings({ currentUserRating: -2, otherRatings: [] });
 
-      expect(component['ratingByAuthor']().get('Alice Reviewer')).toBe(-2);
+      expect(component['ratingByAuthorId']().get('reviewer-1')).toBe(-2);
     });
   });
 });