[#12048] Refactor FeedbackResponses and FeedbackResponseComments attributes

src/e2e/java/teammates/e2e/cases/sql/BaseE2ETestCase.java

@@ -254,7 +254,10 @@ FeedbackResponseData getFeedbackResponse(String questionId, String giver, String
 
     @Override
     protected FeedbackResponseData getFeedbackResponse(FeedbackResponse fr) {
-        return getFeedbackResponse(fr.getFeedbackQuestion().getId().toString(), fr.getGiver(), fr.getRecipient());
+        return getFeedbackResponse(
+                fr.getFeedbackQuestion().getId().toString(),
+                fr.getGiver().getEmail(),
+                fr.getRecipient().getEmail());
     }
 
     StudentData getStudent(String courseId, String studentEmailAddress) {

src/e2e/java/teammates/e2e/cases/sql/FeedbackMcqQuestionE2ETest.java

@@ -134,7 +134,6 @@ private FeedbackResponse getResponse(FeedbackQuestion feedbackQuestion, boolean
         } else {
             details.setAnswer(answer);
         }
-        return FeedbackResponse.makeResponse(feedbackQuestion, student.getEmail(), null, instructor.getEmail(), null,
-                details);
+        return FeedbackResponse.makeResponse(feedbackQuestion, student, null, instructor, null, details);
     }
 }

src/e2e/java/teammates/e2e/cases/sql/FeedbackMsqQuestionE2ETest.java

@@ -144,7 +144,7 @@ private FeedbackResponse getResponse(FeedbackQuestion feedbackQuestion, Student
         }
         details.setAnswers(answers);
 
-        return FeedbackResponse.makeResponse(feedbackQuestion, student.getEmail(), student.getSection(),
-                receiver.getEmail(), receiver.getSection(), details);
+        return FeedbackResponse.makeResponse(feedbackQuestion, student, student.getSection(),
+                receiver, receiver.getSection(), details);
     }
 }

src/e2e/java/teammates/e2e/cases/sql/FeedbackRankOptionQuestionE2ETest.java

@@ -123,6 +123,6 @@ protected void testSubmitPage() {
     private FeedbackResponse getResponse(FeedbackQuestion question, Student receiver, List<Integer> answers) {
         FeedbackRankOptionsResponseDetails details = new FeedbackRankOptionsResponseDetails();
         details.setAnswers(answers);
-        return FeedbackResponse.makeResponse(question, student.getEmail(), null, receiver.getEmail(), null, details);
+        return FeedbackResponse.makeResponse(question, student, null, receiver, null, details);
     }
 }

src/e2e/java/teammates/e2e/cases/sql/FeedbackTextQuestionE2ETest.java

@@ -106,6 +106,6 @@ protected void testSubmitPage() {
     private FeedbackResponse getResponse(FeedbackQuestion feedbackQuestion, Instructor instructor, String answer) {
         FeedbackTextResponseDetails details = new FeedbackTextResponseDetails(answer);
         return FeedbackResponse.makeResponse(
-            feedbackQuestion, student.getEmail(), null, instructor.getEmail(), null, details);
+            feedbackQuestion, student, null, instructor, null, details);
     }
 }

src/it/java/teammates/it/sqllogic/core/DataBundleLogicIT.java

@@ -192,17 +192,18 @@ public void testCreateDataBundle_typicalValues_createdCorrectly() throws Excepti
         ______TS("verify feedback responses deserialized correctly");
         FeedbackResponse actualResponse1 = dataBundle.feedbackResponses.get("response1ForQ1S1C1");
         FeedbackResponseDetails responseDetails1 = new FeedbackTextResponseDetails("Student 1 self feedback.");
-        FeedbackResponse expectedResponse1 = FeedbackResponse.makeResponse(actualQuestion1, "[email protected]",
-                expectedSection, "[email protected]", expectedSection, responseDetails1);
+        FeedbackResponse expectedResponse1 = FeedbackResponse.makeResponse(actualQuestion1, actualStudent1,
+                expectedSection, actualStudent1, expectedSection, responseDetails1);
         expectedResponse1.setId(actualResponse1.getId());
         verifyEquals(expectedResponse1, actualResponse1);
 
         ______TS("verify feedback response comments deserialized correctly");
         FeedbackResponseComment actualComment1 = dataBundle.feedbackResponseComments.get("comment1ToResponse1ForQ1");
-        FeedbackResponseComment expectedComment1 = new FeedbackResponseComment(expectedResponse1, "[email protected]",
+        Instructor expectedInstructor = dataBundle.instructors.get("instructor1OfTypicalCourse");
+        FeedbackResponseComment expectedComment1 = new FeedbackResponseComment(expectedResponse1, expectedInstructor,
                 FeedbackParticipantType.INSTRUCTORS, expectedSection, expectedSection,
                 "Instructor 1 comment to student 1 self feedback", false, false,
-                new ArrayList<FeedbackParticipantType>(), new ArrayList<FeedbackParticipantType>(), "[email protected]");
+                new ArrayList<FeedbackParticipantType>(), new ArrayList<FeedbackParticipantType>(), expectedInstructor);
         expectedComment1.setId(actualComment1.getId());
         verifyEquals(expectedComment1, actualComment1);
     }

src/it/java/teammates/it/sqllogic/core/FeedbackResponsesLogicIT.java

@@ -2,6 +2,7 @@
 
 import java.time.Instant;
 import java.util.List;
+import java.util.UUID;
 
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.BeforeMethod;
@@ -15,6 +16,7 @@
 import teammates.storage.sqlentity.FeedbackResponse;
 import teammates.storage.sqlentity.FeedbackResponseComment;
 import teammates.storage.sqlentity.Section;
+import teammates.storage.sqlentity.User;
 
 /**
  * SUT: {@link FeedbackResponsesLogic}.
@@ -64,7 +66,9 @@ public void testUpdatedFeedbackResponsesAndCommentsCascade() throws Exception {
 
         Section newGiverSection = typicalDataBundle.sections.get("section1InCourse2");
         Section newRecipientSection = typicalDataBundle.sections.get("section2InCourse1");
-        String newGiver = "new test giver";
+        User newGiver = typicalDataBundle.students.get("student1InCourse1");
+        // As assertion uses UUID to compare, let's change the UUID.
+        newGiver.setId(UUID.randomUUID());
 
         for (FeedbackResponseComment frc : oldComments) {
             assertNotEquals(frc.getGiverSection(), newGiverSection);

src/it/java/teammates/it/storage/sqlapi/FeedbackResponseCommentsDbIT.java

@@ -59,66 +59,6 @@ public void testGetFeedbackResponseCommentForResponseFromParticipant() {
         assertEquals(expectedComment, actualComment);
     }
 
-    private FeedbackResponseComment prepareSqlInjectionTest() {
-        FeedbackResponseComment frc = testDataBundle.feedbackResponseComments.get("comment1ToResponse1ForQ1");
-        assertNotNull(frcDb.getFeedbackResponseComment(frc.getId()));
-
-        return frc;
-    }
-
-    private void checkSqlInjectionFailed(FeedbackResponseComment frc) {
-        assertNotNull(frcDb.getFeedbackResponseComment(frc.getId()));
-    }
-
-    @Test
-    public void testSqlInjectionInUpdateGiverEmailOfFeedbackResponseComments() {
-        FeedbackResponseComment frc = prepareSqlInjectionTest();
-
-        String sqli = "'; DELETE FROM feedback_response_comments;--";
-        frcDb.updateGiverEmailOfFeedbackResponseComments(sqli, "", "");
-
-        checkSqlInjectionFailed(frc);
-    }
-
-    @Test
-    public void testSqlInjectionInUpdateLastEditorEmailOfFeedbackResponseComments() {
-        FeedbackResponseComment frc = prepareSqlInjectionTest();
-
-        String sqli = "'; DELETE FROM feedback_response_comments;--";
-        frcDb.updateLastEditorEmailOfFeedbackResponseComments(sqli, "", "");
-
-        checkSqlInjectionFailed(frc);
-    }
-
-    @Test
-    public void testSqlInjectionInCreateFeedbackResponseComment() throws Exception {
-        FeedbackResponseComment frc = prepareSqlInjectionTest();
-
-        FeedbackResponse fr = testDataBundle.feedbackResponses.get("response1ForQ1");
-        Section s = testDataBundle.sections.get("section2InCourse1");
-
-        String sqli = "'');/**/DELETE/**/FROM/**/feedback_response_comments;[email protected]";
-        FeedbackResponseComment newFrc = new FeedbackResponseComment(
-                fr, "", FeedbackParticipantType.INSTRUCTORS, s, s, "",
-                false, false,
-                new ArrayList<FeedbackParticipantType>(), new ArrayList<FeedbackParticipantType>(), sqli);
-
-        frcDb.createFeedbackResponseComment(newFrc);
-
-        checkSqlInjectionFailed(frc);
-    }
-
-    @Test
-    public void testSqlInjectionInUpdateFeedbackResponseComment() throws Exception {
-        FeedbackResponseComment frc = prepareSqlInjectionTest();
-
-        String sqli = "'');/**/DELETE/**/FROM/**/feedback_response_comments;[email protected]";
-        frc.setLastEditorEmail(sqli);
-        frcDb.updateFeedbackResponseComment(frc);
-
-        checkSqlInjectionFailed(frc);
-    }
-
     @Test
     public void testGetFeedbackResponseCommentsForSession_matchFound_success() {
         Course course = testDataBundle.courses.get("course1");

src/it/java/teammates/it/storage/sqlapi/FeedbackResponsesDbIT.java

@@ -159,7 +159,7 @@ public void testSqlInjectionInGetFeedbackResponsesFromGiverForCourse() {
 
         ______TS("SQL Injection test in GetFeedbackResponsesFromGiverForCourse, courseId param");
         String courseId = "'; DELETE FROM feedback_responses;--";
-        frDb.getFeedbackResponsesFromGiverForCourse(courseId, "");
+        frDb.getFeedbackResponsesFromGiverForCourse(courseId, null);
 
         checkSqliFailed(fr);
     }
@@ -170,7 +170,7 @@ public void testSqlInjectionInGetFeedbackResponsesForRecipientForCourse() {
 
         ______TS("SQL Injection test in GetFeedbackResponsesForRecipientForCourse, courseId param");
         String courseId = "'; DELETE FROM feedback_responses;--";
-        frDb.getFeedbackResponsesForRecipientForCourse(courseId, "");
+        frDb.getFeedbackResponsesForRecipientForCourse(courseId, null);
 
         checkSqliFailed(fr);
     }
@@ -208,34 +208,6 @@ public void testSqlInjectionInHasResponsesForCourse() {
         checkSqliFailed(fr);
     }
 
-    @Test
-    public void testSqlInjectionInCreateFeedbackResponse() throws Exception {
-        FeedbackResponse fr = prepareSqlInjectionTest();
-
-        FeedbackQuestion fq = testDataBundle.feedbackQuestions.get("qn1InSession1InCourse1");
-        Section s = testDataBundle.sections.get("section1InCourse1");
-        String dummyUuid = "00000000-0000-4000-8000-000000000001";
-        FeedbackResponseDetails frd = new FeedbackTextResponseDetails();
-
-        String sqli = "', " + dummyUuid + ", " + dummyUuid + "); DELETE FROM feedback_responses;--";
-
-        FeedbackResponse newFr = new FeedbackTextResponse(fq, "", s, sqli, s, frd);
-        frDb.createFeedbackResponse(newFr);
-
-        checkSqliFailed(fr);
-    }
-
-    @Test
-    public void testSqlInjectionInCpdateFeedbackResponse() throws Exception {
-        FeedbackResponse fr = prepareSqlInjectionTest();
-
-        String sqli = "''); DELETE FROM feedback_response_comments;--";
-        fr.setGiver(sqli);
-        frDb.updateFeedbackResponse(fr);
-
-        checkSqliFailed(fr);
-    }
-
     @Test
     public void testGetFeedbackResponsesForRecipientForQuestion_matchNotFound_shouldReturnEmptyList() {
         ______TS("Question not found");

src/it/java/teammates/it/ui/webapi/CreateFeedbackResponseCommentActionIT.java

@@ -61,7 +61,7 @@ protected void testExecute() throws Exception {
         FeedbackResponseComment comment =
                 logic.getFeedbackResponseCommentForResponseFromParticipant(fr.getId());
         assertEquals(comment.getCommentText(), "Student submission comment");
-        assertEquals(student.getEmail(), comment.getGiver());
+        assertEquals(student.getEmail(), comment.getGiver().getEmail());
         assertTrue(comment.getIsCommentFromFeedbackParticipant());
         assertTrue(comment.getIsVisibilityFollowingFeedbackQuestion());
         assertEquals(FeedbackParticipantType.STUDENTS, comment.getGiverType());

src/it/java/teammates/it/ui/webapi/DeleteFeedbackResponseCommentActionIT.java

@@ -64,14 +64,14 @@ protected void testAccessControl() throws Exception {
         };
         Instructor instructorWhoGiveComment = typicalBundle.instructors.get("instructor1OfCourse1");
 
-        assertEquals(instructorWhoGiveComment.getEmail(), frc.getGiver());
+        assertEquals(instructorWhoGiveComment.getEmail(), frc.getGiver().getEmail());
         loginAsInstructor(instructorWhoGiveComment.getGoogleId());
         verifyCanAccess(submissionParams);
 
         ______TS("Different instructor of same course cannot delete comment");
 
         Instructor differentInstructorInSameCourse = typicalBundle.instructors.get("instructor2OfCourse1");
-        assertNotEquals(differentInstructorInSameCourse.getEmail(), frc.getGiver());
+        assertNotEquals(differentInstructorInSameCourse.getEmail(), frc.getGiver().getEmail());
         loginAsInstructor(differentInstructorInSameCourse.getGoogleId());
         verifyCannotAccess(submissionParams);
     }

src/it/java/teammates/it/ui/webapi/EnrollStudentsActionIT.java

@@ -130,7 +130,7 @@ public void testExecute() throws Exception {
             assertEquals(logic.getSection(courseId, "Section 3"), response.getRecipientSection());
             List<FeedbackResponseComment> commentsFromUser = logic.getFeedbackResponseCommentsForResponse(response.getId());
             for (FeedbackResponseComment comment : commentsFromUser) {
-                if (comment.getGiver().equals(giverEmail)) {
+                if (comment.getGiver().getEmail().equals(giverEmail)) {
                     assertEquals(logic.getSection(courseId, "Section 3"), comment.getGiverSection());
                 }
             }

src/it/java/teammates/it/ui/webapi/UpdateFeedbackResponseCommentActionIT.java

@@ -59,7 +59,7 @@ protected void testExecute() throws Exception {
 
         FeedbackResponseComment actualFrc = logic.getFeedbackResponseComment(frc.getId());
         assertEquals(newCommentText, actualFrc.getCommentText());
-        assertEquals(instructor.getEmail(), actualFrc.getLastEditorEmail());
+        assertEquals(instructor.getEmail(), actualFrc.getLastEditor().getEmail());
     }
 
     @Test

src/it/resources/data/DataBundleLogicIT.json

@@ -257,8 +257,14 @@
           "INSTRUCTORS"
         ]
       },
-      "giver": "[email protected]",
-      "recipient": "[email protected]",
+      "giver": {
+        "id": "00000000-0000-4000-8000-000000000601",
+        "type": "student"
+      },
+      "recipient": {
+        "id": "00000000-0000-4000-8000-000000000601",
+        "type": "student"
+      },
       "giverSection": {
         "id": "00000000-0000-4000-8000-000000000201"
       },
@@ -299,8 +305,14 @@
             "INSTRUCTORS"
           ]
         },
-        "giver": "[email protected]",
-        "recipient": "[email protected]",
+        "giver": {
+          "id": "00000000-0000-4000-8000-000000000601",
+          "type": "student"
+        },
+        "recipient": {
+          "id": "00000000-0000-4000-8000-000000000601",
+          "type": "student"
+        },
         "giverSection": {
           "id": "00000000-0000-4000-8000-000000000201"
         },
@@ -312,7 +324,11 @@
           "answer": "Student 1 self feedback."
         }
       },
-      "giver": "[email protected]",
+      "giver": {
+        "id": "00000000-0000-4000-8000-000000000501",
+        "type": "instructor",
+        "email": "[email protected]"
+      },
       "giverType": "INSTRUCTORS",
       "giverSection": {
         "id": "00000000-0000-4000-8000-000000000201"
@@ -325,7 +341,11 @@
       "isCommentFromFeedbackParticipant": false,
       "showCommentTo": [],
       "showGiverNameTo": [],
-      "lastEditorEmail": "[email protected]"
+      "lastEditor": {
+        "id": "00000000-0000-4000-8000-000000000501",
+        "type": "instructor",
+        "email": "[email protected]"
+      }
     }
   },
   "notifications": {