From e1955b6ba5cc81ec2df254faa0ba5c673eb2f5f6 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Mon, 12 Feb 2024 15:34:51 +0800 Subject: [PATCH 01/42] Migrate SessionResultsData --- .../datatransfer/SqlSessionResultsBundle.java | 157 +++++++ .../questions/FeedbackQuestionDetails.java | 14 + .../ui/output/SessionResultsData.java | 389 +++++++++++++++++- 3 files changed, 552 insertions(+), 8 deletions(-) create mode 100644 src/main/java/teammates/common/datatransfer/SqlSessionResultsBundle.java diff --git a/src/main/java/teammates/common/datatransfer/SqlSessionResultsBundle.java b/src/main/java/teammates/common/datatransfer/SqlSessionResultsBundle.java new file mode 100644 index 00000000000..e4693818876 --- /dev/null +++ b/src/main/java/teammates/common/datatransfer/SqlSessionResultsBundle.java @@ -0,0 +1,157 @@ +package teammates.common.datatransfer; + +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import teammates.common.util.Const; +import teammates.common.util.StringHelper; +import teammates.storage.sqlentity.FeedbackQuestion; +import teammates.storage.sqlentity.FeedbackResponse; +import teammates.storage.sqlentity.FeedbackResponseComment; + +/** + * Represents detailed results for a feedback session. + */ +public class SqlSessionResultsBundle { + + private final List questions; + private final Map> questionResponseMap; + private final Map> questionMissingResponseMap; + private final Map> responseCommentsMap; + private final Map responseGiverVisibilityTable; + private final Map responseRecipientVisibilityTable; + private final Map commentGiverVisibilityTable; + private final SqlCourseRoster roster; + + public SqlSessionResultsBundle(List questions, + List responses, + List missingResponses, + Map responseGiverVisibilityTable, + Map responseRecipientVisibilityTable, + Map> responseCommentsMap, + Map commentGiverVisibilityTable, + SqlCourseRoster roster) { + + this.questions = questions; + this.responseCommentsMap = responseCommentsMap; + this.responseGiverVisibilityTable = responseGiverVisibilityTable; + this.responseRecipientVisibilityTable = responseRecipientVisibilityTable; + this.commentGiverVisibilityTable = commentGiverVisibilityTable; + this.roster = roster; + this.questionResponseMap = buildQuestionToResponseMap(responses); + this.questionMissingResponseMap = buildQuestionToResponseMap(missingResponses); + } + + private Map> buildQuestionToResponseMap( + List responses) { + // build question to response map + Map> questionToResponseMap = new LinkedHashMap<>(); + for (FeedbackQuestion question : questions) { + questionToResponseMap.put(question, new ArrayList<>()); + } + for (FeedbackResponse response : responses) { + FeedbackQuestion question = response.getFeedbackQuestion(); + List responsesForQuestion = questionToResponseMap.get(question.getId()); + responsesForQuestion.add(response); + } + return questionToResponseMap; + } + + /** + * Returns true if the giver of a response is visible to the current user. + * Returns false otherwise. + */ + public boolean isResponseGiverVisible(FeedbackResponse response) { + return isResponseParticipantVisible(true, response); + } + + /** + * Returns true if the recipient of a response is visible to the current user. + * Returns false otherwise. + */ + public boolean isResponseRecipientVisible(FeedbackResponse response) { + return isResponseParticipantVisible(false, response); + } + + /** + * Checks if the giver/recipient for a response is visible/hidden from the current user. + */ + private boolean isResponseParticipantVisible(boolean isGiver, FeedbackResponse response) { + FeedbackQuestion question = response.getFeedbackQuestion(); + FeedbackParticipantType participantType; + + boolean isVisible; + if (isGiver) { + isVisible = responseGiverVisibilityTable.get(response); + participantType = question.getGiverType(); + } else { + isVisible = responseRecipientVisibilityTable.get(response); + participantType = question.getRecipientType(); + } + boolean isTypeNone = participantType == FeedbackParticipantType.NONE; + + return isVisible || isTypeNone; + } + + /** + * Returns true if the giver of a comment is visible to the current user. + * Returns false otherwise. + */ + public boolean isCommentGiverVisible(FeedbackResponseComment comment) { + return commentGiverVisibilityTable.get(comment.getId()); + } + + /** + * Gets the anonymous name for a given name. + * + *

The anonymous name will be deterministic based on {@code name}. + */ + public static String getAnonName(FeedbackParticipantType type, String name) { + String hashedEncryptedName = getHashOfName(getEncryptedName(name)); + String participantType = type.toSingularFormString(); + return String.format( + Const.DISPLAYED_NAME_FOR_ANONYMOUS_PARTICIPANT + " %s %s", participantType, hashedEncryptedName); + } + + public Map> getQuestionResponseMap() { + return questionResponseMap; + } + + public Map> getQuestionMissingResponseMap() { + return questionMissingResponseMap; + } + + private static String getEncryptedName(String name) { + return StringHelper.encrypt(name); + } + + private static String getHashOfName(String name) { + return Long.toString(Math.abs((long) name.hashCode())); + } + + public List getQuestions() { + return questions; + } + + public Map> getResponseCommentsMap() { + return responseCommentsMap; + } + + public SqlCourseRoster getRoster() { + return roster; + } + + public Map getResponseGiverVisibilityTable() { + return responseGiverVisibilityTable; + } + + public Map getResponseRecipientVisibilityTable() { + return responseRecipientVisibilityTable; + } + + public Map getCommentGiverVisibilityTable() { + return commentGiverVisibilityTable; + } +} diff --git a/src/main/java/teammates/common/datatransfer/questions/FeedbackQuestionDetails.java b/src/main/java/teammates/common/datatransfer/questions/FeedbackQuestionDetails.java index 0e825885017..573193dfd0b 100644 --- a/src/main/java/teammates/common/datatransfer/questions/FeedbackQuestionDetails.java +++ b/src/main/java/teammates/common/datatransfer/questions/FeedbackQuestionDetails.java @@ -4,6 +4,7 @@ import teammates.common.datatransfer.FeedbackParticipantType; import teammates.common.datatransfer.SessionResultsBundle; +import teammates.common.datatransfer.SqlSessionResultsBundle; import teammates.common.datatransfer.attributes.FeedbackQuestionAttributes; import teammates.common.util.JsonUtils; import teammates.storage.sqlentity.FeedbackQuestion; @@ -41,6 +42,19 @@ public String getQuestionResultStatisticsJson( return ""; } + /** + * Get question result statistics as JSON string. + */ + @SuppressWarnings("PMD.EmptyMethodInAbstractClassShouldBeAbstract") + public String getQuestionResultStatisticsJson( + FeedbackQuestion question, String studentEmail, SqlSessionResultsBundle bundle) { + // Statistics are calculated in the front-end as it is dependent on the responses being filtered. + // The only exception is contribution question, where there is only one statistics for the entire question. + // It is also necessary to calculate contribution question statistics here + // to be displayed in student result page as students are not supposed to be able to see the exact responses. + return ""; + } + /** * Checks whether the changes to the question details require deletion of corresponding responses. */ diff --git a/src/main/java/teammates/ui/output/SessionResultsData.java b/src/main/java/teammates/ui/output/SessionResultsData.java index b55be5cf9a5..81045916899 100644 --- a/src/main/java/teammates/ui/output/SessionResultsData.java +++ b/src/main/java/teammates/ui/output/SessionResultsData.java @@ -13,6 +13,8 @@ import teammates.common.datatransfer.CourseRoster; import teammates.common.datatransfer.FeedbackParticipantType; import teammates.common.datatransfer.SessionResultsBundle; +import teammates.common.datatransfer.SqlCourseRoster; +import teammates.common.datatransfer.SqlSessionResultsBundle; import teammates.common.datatransfer.attributes.FeedbackQuestionAttributes; import teammates.common.datatransfer.attributes.FeedbackResponseAttributes; import teammates.common.datatransfer.attributes.FeedbackResponseCommentAttributes; @@ -22,6 +24,13 @@ import teammates.common.datatransfer.questions.FeedbackResponseDetails; import teammates.common.util.Const; import teammates.common.util.StringHelper; +import teammates.storage.sqlentity.FeedbackQuestion; +import teammates.storage.sqlentity.FeedbackResponse; +import teammates.storage.sqlentity.FeedbackResponseComment; +import teammates.storage.sqlentity.Instructor; +import teammates.storage.sqlentity.Section; +import teammates.storage.sqlentity.Student; +import teammates.ui.output.SessionResultsData.CommentOutput; /** * API output format for session results, including statistics. @@ -64,6 +73,34 @@ public static SessionResultsData initForInstructor(SessionResultsBundle bundle) return sessionResultsData; } + + /** + * Factory method to construct API output for instructor. + */ + public static SessionResultsData initForInstructor(SqlSessionResultsBundle bundle) { + SessionResultsData sessionResultsData = new SessionResultsData(); + + Map> questionsWithResponses = + bundle.getQuestionResponseMap(); + + questionsWithResponses.forEach((question, responses) -> { + FeedbackQuestionDetails questionDetails = question.getQuestionDetailsCopy(); + QuestionOutput qnOutput = new QuestionOutput(question, + questionDetails.getQuestionResultStatisticsJson(question, null, bundle)); + // put normal responses + List allResponses = buildResponsesForInstructor(responses, bundle, false); + qnOutput.allResponses.addAll(allResponses); + + // put missing responses + List missingResponses = bundle.getQuestionMissingResponseMap().get(question); + qnOutput.allResponses.addAll(buildResponsesForInstructor(missingResponses, bundle, true)); + + sessionResultsData.questions.add(qnOutput); + }); + + return sessionResultsData; + } + /** * Factory method to construct API output for student. */ @@ -120,6 +157,62 @@ public static SessionResultsData initForStudent(SessionResultsBundle bundle, Stu return sessionResultsData; } + + /** + * Factory method to construct API output for student. + */ + public static SessionResultsData initForStudent(SqlSessionResultsBundle bundle, Student student) { + SessionResultsData sessionResultsData = new SessionResultsData(); + + Map> questionsWithResponses = + bundle.getQuestionResponseMap(); + + questionsWithResponses.forEach((question, responses) -> { + FeedbackQuestionDetails questionDetails = question.getQuestionDetailsCopy(); + QuestionOutput qnOutput = new QuestionOutput(question, + questionDetails.getQuestionResultStatisticsJson(question, student.getEmail(), bundle)); + Map> otherResponsesMap = new HashMap<>(); + + qnOutput.getFeedbackQuestion().hideInformationForStudent(); + + if (questionDetails.isIndividualResponsesShownToStudents()) { + for (FeedbackResponse response : responses) { + boolean isUserInstructor = Const.USER_TEAM_FOR_INSTRUCTOR.equals(student.getTeam()); + + boolean isUserGiver = student.getEmail().equals(response.getGiver()) + && (isUserInstructor && question.getGiverType() == FeedbackParticipantType.INSTRUCTORS + || !isUserInstructor && question.getGiverType() != FeedbackParticipantType.INSTRUCTORS); + boolean isUserRecipient = student.getEmail().equals(response.getRecipient()) + && (isUserInstructor && question.getRecipientType() == FeedbackParticipantType.INSTRUCTORS + || !isUserInstructor && question.getRecipientType() != FeedbackParticipantType.INSTRUCTORS); + ResponseOutput responseOutput = buildSingleResponseForStudent(response, bundle, student); + + if (isUserRecipient) { + qnOutput.responsesToSelf.add(responseOutput); + } + + if (isUserGiver) { + qnOutput.responsesFromSelf.add(responseOutput); + } + + if (!isUserRecipient && !isUserGiver) { + // we don't need care about the keys of the map here + // as only the values of the map will be used + otherResponsesMap.computeIfAbsent(response.getRecipient(), k -> new ArrayList<>()) + .add(responseOutput); + } + + qnOutput.allResponses.add(responseOutput); + } + } + qnOutput.otherResponses.addAll(otherResponsesMap.values()); + + sessionResultsData.questions.add(qnOutput); + }); + + return sessionResultsData; + } + private static ResponseOutput buildSingleResponseForStudent( FeedbackResponseAttributes response, SessionResultsBundle bundle, StudentAttributes student) { FeedbackQuestionAttributes question = bundle.getQuestionsMap().get(response.getFeedbackQuestionId()); @@ -178,12 +271,82 @@ private static ResponseOutput buildSingleResponseForStudent( .withGiverTeam(giverTeam) .withGiverEmail(null) .withRelatedGiverEmail(null) + .withGiverSectionName(response.getGiverSection()) + .withRecipient(recipientName) + .withRecipientTeam(recipientTeam) + .withRecipientEmail(null) + .withRecipientSectionName(response.getRecipientSection()) + .withResponseDetails(response.getResponseDetailsCopy()) + .withParticipantComment(comments.poll()) + .withInstructorComments(new ArrayList<>(comments)) + .build(); + } + + + private static ResponseOutput buildSingleResponseForStudent( + FeedbackResponse response, SqlSessionResultsBundle bundle, Student student) { + FeedbackQuestion question = response.getFeedbackQuestion(); + boolean isUserInstructor = Const.USER_TEAM_FOR_INSTRUCTOR.equals(student.getTeam()); + + // process giver + boolean isUserGiver = student.getEmail().equals(response.getGiver()) + && (isUserInstructor && question.getGiverType() == FeedbackParticipantType.INSTRUCTORS + || !isUserInstructor && question.getGiverType() != FeedbackParticipantType.INSTRUCTORS); + boolean isUserTeamGiver = question.getGiverType() == FeedbackParticipantType.TEAMS + && student.getTeam().equals(response.getGiver()); + String giverName; + String giverTeam = ""; + if (isUserTeamGiver) { + giverName = String.format("Your Team (%s)", response.getGiver()); + giverTeam = response.getGiver(); + } else if (isUserGiver) { + giverName = "You"; + giverTeam = student.getTeamName(); + } else { + // we don't want student to figure out who is who by using the hash + giverName = removeAnonymousHash(getGiverNameOfResponse(response, bundle)); + } + + // process recipient + boolean isUserRecipient = student.getEmail().equals(response.getRecipient()) + && (isUserInstructor && question.getRecipientType() == FeedbackParticipantType.INSTRUCTORS + || !isUserInstructor && question.getRecipientType() != FeedbackParticipantType.INSTRUCTORS); + boolean isUserTeamRecipient = (question.getRecipientType() == FeedbackParticipantType.TEAMS + || question.getRecipientType() == FeedbackParticipantType.TEAMS_IN_SAME_SECTION) + && student.getTeam().equals(response.getRecipient()); + String recipientName; + String recipientTeam = ""; + if (isUserRecipient) { + recipientName = "You"; + recipientTeam = student.getTeamName(); + } else if (isUserTeamRecipient) { + recipientName = String.format("Your Team (%s)", response.getRecipient()); + recipientTeam = response.getRecipient(); + } else { + // we don't want student to figure out who is who by using the hash + recipientName = removeAnonymousHash(getRecipientNameOfResponse(response, bundle)); + if (!recipientName.contains(Const.DISPLAYED_NAME_FOR_ANONYMOUS_PARTICIPANT)) { + recipientTeam = bundle.getRoster().getInfoForIdentifier(response.getRecipient()).getTeamName(); + } + } + + // process comments + List feedbackResponseComments = + bundle.getResponseCommentsMap().getOrDefault(response, Collections.emptyList()); + Queue comments = buildComments(feedbackResponseComments, bundle); + + return ResponseOutput.builder() + .withResponse(response) + .withGiver(giverName) + .withGiverTeam(giverTeam) + .withGiverEmail(null) + .withRelatedGiverEmail(null) .withGiverSection(response.getGiverSection()) .withRecipient(recipientName) .withRecipientTeam(recipientTeam) .withRecipientEmail(null) .withRecipientSection(response.getRecipientSection()) - .withResponseDetails(response.getResponseDetailsCopy()) + .withResponseDetails(response.getFeedbackResponseDetailsCopy()) .withParticipantComment(comments.poll()) .withInstructorComments(new ArrayList<>(comments)) .build(); @@ -205,6 +368,18 @@ private static List buildResponsesForInstructor( return output; } + private static List buildResponsesForInstructor( + List responses, SqlSessionResultsBundle bundle, boolean areMissingResponses) { + List output = new ArrayList<>(); + + for (FeedbackResponse response : responses) { + output.add(buildSingleResponseForInstructor(response, bundle, areMissingResponses)); + } + + return output; + } + + private static ResponseOutput buildSingleResponseForInstructor( FeedbackResponseAttributes response, SessionResultsBundle bundle, boolean isMissingResponse) { // process giver @@ -268,17 +443,92 @@ private static ResponseOutput buildSingleResponseForInstructor( .withGiverTeam(giverTeam) .withGiverEmail(giverEmail) .withRelatedGiverEmail(relatedGiverEmail) - .withGiverSection(giverSection) + .withGiverSectionName(giverSection) .withRecipient(recipientName) .withRecipientTeam(recipientTeam) .withRecipientEmail(recipientEmail) - .withRecipientSection(recipientSection) + .withRecipientSectionName(recipientSection) .withResponseDetails(response.getResponseDetailsCopy()) .withParticipantComment(comments.poll()) .withInstructorComments(new ArrayList<>(comments)) .build(); } + + private static ResponseOutput buildSingleResponseForInstructor( + FeedbackResponse response, SqlSessionResultsBundle bundle, boolean isMissingResponse) { + // process giver + String giverEmail = null; + String relatedGiverEmail = null; + if (bundle.isResponseGiverVisible(response)) { + giverEmail = response.getGiver(); + relatedGiverEmail = response.getGiver(); + + if (bundle.getRoster().isTeamInCourse(giverEmail)) { + // remove recipient email as it is a team name + relatedGiverEmail = + bundle.getRoster().getTeamToMembersTable().get(giverEmail).iterator().next().getEmail(); + giverEmail = null; + } + } + String giverName = getGiverNameOfResponse(response, bundle); + String giverTeam = bundle.getRoster().getInfoForIdentifier(response.getGiver()).getTeamName(); + String giverSectionName = response.getGiverSection().getName(); + FeedbackQuestion question = response.getFeedbackQuestion(); + if (question.getGiverType() == FeedbackParticipantType.INSTRUCTORS) { + Instructor instructor = bundle.getRoster().getInstructorForEmail(response.getGiver()); + giverName = instructor.getName(); + giverTeam = Const.USER_TEAM_FOR_INSTRUCTOR; + giverSectionName = Const.DEFAULT_SECTION; + } + + // process recipient + String recipientEmail = null; + String recipientName = getRecipientNameOfResponse(response, bundle); + String recipientTeam = + bundle.getRoster().getInfoForIdentifier(response.getRecipient()).getTeamName(); + String recipientSectionName = response.getRecipientSection().getName(); + if (question.getRecipientType() == FeedbackParticipantType.INSTRUCTORS) { + Instructor instructor = bundle.getRoster().getInstructorForEmail(response.getRecipient()); + recipientName = instructor.getName(); + recipientTeam = Const.USER_TEAM_FOR_INSTRUCTOR; + recipientSectionName = Const.DEFAULT_SECTION; + } + if (bundle.isResponseRecipientVisible(response)) { + recipientEmail = response.getRecipient(); + + if (bundle.getRoster().isTeamInCourse(recipientEmail)) { + // remove recipient email as it is a team name + recipientEmail = null; + } else if (Const.GENERAL_QUESTION.equals(recipientEmail)) { + // general recipient does not have email + recipientEmail = null; + } + } + + // process comments + List feedbackResponseComments = + bundle.getResponseCommentsMap().getOrDefault(response, Collections.emptyList()); + Queue comments = buildComments(feedbackResponseComments, bundle); + + return ResponseOutput.builder() + .withIsMissingResponse(isMissingResponse) + .withResponse(response) + .withGiver(giverName) + .withGiverTeam(giverTeam) + .withGiverEmail(giverEmail) + .withRelatedGiverEmail(relatedGiverEmail) + .withGiverSectionName(giverSectionName) + .withRecipient(recipientName) + .withRecipientTeam(recipientTeam) + .withRecipientEmail(recipientEmail) + .withRecipientSectionName(recipientSectionName) + .withResponseDetails(response.getFeedbackResponseDetailsCopy()) + .withParticipantComment(comments.poll()) + .withInstructorComments(new ArrayList<>(comments)) + .build(); + } + /** * Gets giver name of a response from the bundle. * @@ -298,6 +548,26 @@ private static String getGiverNameOfResponse(FeedbackResponseAttributes response return name; } + + /** + * Gets giver name of a response from the bundle. + * + *

Anonymized the name if necessary. + */ + private static String getGiverNameOfResponse(FeedbackResponse response, SqlSessionResultsBundle bundle) { + FeedbackQuestion question = response.getFeedbackQuestion(); + FeedbackParticipantType participantType = question.getGiverType(); + + SqlCourseRoster.ParticipantInfo userInfo = bundle.getRoster().getInfoForIdentifier(response.getGiver()); + String name = userInfo.getName(); + + if (!bundle.isResponseGiverVisible(response)) { + name = SessionResultsBundle.getAnonName(participantType, name); + } + + return name; + } + /** * Gets recipient name of a response from the bundle. * @@ -324,6 +594,33 @@ private static String getRecipientNameOfResponse(FeedbackResponseAttributes resp return name; } + + /** + * Gets recipient name of a response from the bundle. + * + *

Anonymized the name if necessary. + */ + private static String getRecipientNameOfResponse(FeedbackResponse response, SqlSessionResultsBundle bundle) { + FeedbackQuestion question = response.getFeedbackQuestion(); + FeedbackParticipantType participantType = question.getRecipientType(); + if (participantType == FeedbackParticipantType.SELF) { + // recipient type for self-feedback is the same as the giver type + participantType = question.getGiverType(); + } + + SqlCourseRoster.ParticipantInfo userInfo = bundle.getRoster().getInfoForIdentifier(response.getRecipient()); + String name = userInfo.getName(); + if (Const.GENERAL_QUESTION.equals(response.getRecipient())) { + // for general question + name = Const.USER_NOBODY_TEXT; + } + if (!bundle.isResponseRecipientVisible(response)) { + name = SessionResultsBundle.getAnonName(participantType, name); + } + + return name; + } + private static Queue buildComments(List feedbackResponseComments, SessionResultsBundle bundle) { LinkedList outputs = new LinkedList<>(); @@ -362,6 +659,45 @@ private static Queue buildComments(List buildComments(List feedbackResponseComments, + SqlSessionResultsBundle bundle) { + LinkedList outputs = new LinkedList<>(); + + CommentOutput participantComment = null; + for (FeedbackResponseComment comment : feedbackResponseComments) { + if (comment.getIsCommentFromFeedbackParticipant()) { + // participant comment will not need these fields + participantComment = CommentOutput.builder(comment) + .withCommentGiver(null) + .withCommentGiverName(null) + .withLastEditorEmail(null) + .withLastEditorName(null) + .build(); + } else { + String giverEmail = Const.DISPLAYED_NAME_FOR_ANONYMOUS_PARTICIPANT; + String giverName = Const.DISPLAYED_NAME_FOR_ANONYMOUS_PARTICIPANT; + String lastEditorEmail = Const.DISPLAYED_NAME_FOR_ANONYMOUS_PARTICIPANT; + String lastEditorName = Const.DISPLAYED_NAME_FOR_ANONYMOUS_PARTICIPANT; + if (bundle.isCommentGiverVisible(comment)) { + giverEmail = comment.getGiver(); + giverName = bundle.getRoster().getInfoForIdentifier(comment.getGiver()).getName(); + lastEditorEmail = comment.getLastEditorEmail(); + lastEditorName = bundle.getRoster().getInfoForIdentifier(comment.getLastEditorEmail()).getName(); + } + outputs.add(CommentOutput.builder(comment) + .withCommentGiver(giverEmail) + .withCommentGiverName(giverName) + .withLastEditorEmail(lastEditorEmail) + .withLastEditorName(lastEditorName) + .build()); + } + } + outputs.addFirst(participantComment); + + return outputs; + } + public List getQuestions() { return questions; } @@ -386,6 +722,11 @@ private QuestionOutput(FeedbackQuestionAttributes feedbackQuestionAttributes, St this.questionStatistics = questionStatistics; } + private QuestionOutput(FeedbackQuestion feedbackQuestion, String questionStatistics) { + this.feedbackQuestion = new FeedbackQuestionData(feedbackQuestion); + this.questionStatistics = questionStatistics; + } + public FeedbackQuestionData getFeedbackQuestion() { return feedbackQuestion; } @@ -535,6 +876,11 @@ Builder withResponseId(String responseId) { return this; } + Builder withResponse(FeedbackResponse response) { + responseOutput.responseId = StringHelper.encrypt(response.getId().toString()); + return this; + } + Builder withGiver(String giverName) { responseOutput.giver = giverName; return this; @@ -555,11 +901,16 @@ Builder withGiverEmail(@Nullable String giverEmail) { return this; } - Builder withGiverSection(String giverSection) { + Builder withGiverSectionName(String giverSection) { responseOutput.giverSection = giverSection; return this; } + Builder withGiverSection(Section giverSection) { + responseOutput.giverSection = giverSection.getName(); + return this; + } + Builder withRecipient(String recipientName) { responseOutput.recipient = recipientName; return this; @@ -575,11 +926,16 @@ Builder withRecipientEmail(@Nullable String recipientEmail) { return this; } - Builder withRecipientSection(String recipientSection) { + Builder withRecipientSectionName(String recipientSection) { responseOutput.recipientSection = recipientSection; return this; } + Builder withRecipientSection(Section recipientSection) { + responseOutput.recipientSection = recipientSection.getName(); + return this; + } + Builder withResponseDetails(FeedbackResponseDetails responseDetails) { responseOutput.responseDetails = responseDetails; return this; @@ -611,7 +967,12 @@ public static class CommentOutput extends FeedbackResponseCommentData { @Nullable private String lastEditorName; - private CommentOutput(FeedbackResponseCommentAttributes frc) { + private CommentOutput(FeedbackResponseCommentAttributes frca) { + // use builder instead + super(frca); + } + + private CommentOutput(FeedbackResponseComment frc) { // use builder instead super(frc); } @@ -619,10 +980,18 @@ private CommentOutput(FeedbackResponseCommentAttributes frc) { /** * Returns a builder for {@link CommentOutput}. */ - static Builder builder(FeedbackResponseCommentAttributes frc) { + static Builder builder(FeedbackResponseCommentAttributes frca) { + return new Builder(frca); + } + + /** + * Returns a builder for {@link CommentOutput}. + */ + static Builder builder(FeedbackResponseComment frc) { return new Builder(frc); } + @Nullable public String getCommentGiverName() { return commentGiverName; @@ -639,7 +1008,11 @@ public String getLastEditorName() { public static final class Builder { private final CommentOutput commentOutput; - private Builder(FeedbackResponseCommentAttributes frc) { + private Builder(FeedbackResponseCommentAttributes frca) { + commentOutput = new CommentOutput(frca); + } + + private Builder(FeedbackResponseComment frc) { commentOutput = new CommentOutput(frc); } From 8f0f835586d892da4d79f23e6ff591c1216e47b4 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Mon, 12 Feb 2024 17:59:53 +0800 Subject: [PATCH 02/42] Add default entities --- src/main/java/teammates/common/util/Const.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/teammates/common/util/Const.java b/src/main/java/teammates/common/util/Const.java index 01839aa197f..c999a9d42c7 100644 --- a/src/main/java/teammates/common/util/Const.java +++ b/src/main/java/teammates/common/util/Const.java @@ -5,6 +5,9 @@ import java.time.Duration; import java.time.Instant; +import teammates.storage.sqlentity.Section; +import teammates.storage.sqlentity.Team; + /** * Stores constants that are widely used across classes. * this class contains several nested classes, each containing a specific @@ -17,6 +20,7 @@ public final class Const { public static final String USER_NOBODY_TEXT = "-"; public static final String USER_TEAM_FOR_INSTRUCTOR = "Instructors"; + public static final Team USER_TEAM_ENTITY_FOR_INSTRUCTOR = new Team(null, USER_TEAM_FOR_INSTRUCTOR); public static final String DEFAULT_DISPLAY_NAME_FOR_INSTRUCTOR = "Instructor"; @@ -25,6 +29,7 @@ public final class Const { public static final int SECTION_SIZE_LIMIT = 100; public static final String DEFAULT_SECTION = "None"; + public static final Section DEFAULT_SECTION_ENTITY = new Section(null, DEFAULT_SECTION); public static final String UNKNOWN_INSTITUTION = "Unknown Institution"; From d40f2fbe2d291735d369b8f678cc4796de7edec8 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Mon, 12 Feb 2024 18:01:34 +0800 Subject: [PATCH 03/42] Add helper methods to assist migrated logic --- .../common/datatransfer/SqlCourseRoster.java | 42 ++++++++++++------- .../storage/sqlentity/FeedbackQuestion.java | 8 ++++ .../sqlentity/FeedbackResponseComment.java | 4 ++ 3 files changed, 38 insertions(+), 16 deletions(-) diff --git a/src/main/java/teammates/common/datatransfer/SqlCourseRoster.java b/src/main/java/teammates/common/datatransfer/SqlCourseRoster.java index 922b9d6c8c6..83b8021cfb2 100644 --- a/src/main/java/teammates/common/datatransfer/SqlCourseRoster.java +++ b/src/main/java/teammates/common/datatransfer/SqlCourseRoster.java @@ -7,7 +7,9 @@ import teammates.common.util.Const; import teammates.storage.sqlentity.Instructor; +import teammates.storage.sqlentity.Section; import teammates.storage.sqlentity.Student; +import teammates.storage.sqlentity.Team; /** * Contains a list of students and instructors in a course. Useful for caching @@ -126,8 +128,8 @@ public static Map> buildTeamToMembersTable(List s */ public ParticipantInfo getInfoForIdentifier(String identifier) { String name = Const.USER_NOBODY_TEXT; - String teamName = Const.USER_NOBODY_TEXT; - String sectionName = Const.DEFAULT_SECTION; + Team team = new Team(null, Const.USER_NOBODY_TEXT); + Section section = Const.DEFAULT_SECTION_ENTITY; boolean isStudent = getStudentForEmail(identifier) != null; boolean isInstructor = getInstructorForEmail(identifier) != null; @@ -136,23 +138,23 @@ public ParticipantInfo getInfoForIdentifier(String identifier) { Student student = getStudentForEmail(identifier); name = student.getName(); - teamName = student.getTeamName(); - sectionName = student.getSectionName(); + team = student.getTeam(); + section = student.getSection(); } else if (isInstructor) { Instructor instructor = getInstructorForEmail(identifier); name = instructor.getName(); - teamName = Const.USER_TEAM_FOR_INSTRUCTOR; - sectionName = Const.DEFAULT_SECTION; + team = Const.USER_TEAM_ENTITY_FOR_INSTRUCTOR; + section = Const.DEFAULT_SECTION_ENTITY; } else if (isTeam) { Student teamMember = getTeamToMembersTable().get(identifier).iterator().next(); name = identifier; - teamName = identifier; - sectionName = teamMember.getSectionName(); + team = new Team(teamMember.getSection(), identifier); + section = teamMember.getSection(); } - return new ParticipantInfo(name, teamName, sectionName); + return new ParticipantInfo(name, team, section); } /** @@ -161,25 +163,33 @@ public ParticipantInfo getInfoForIdentifier(String identifier) { public static class ParticipantInfo { private final String name; - private final String teamName; - private final String sectionName; + private final Team team; + private final Section section; - private ParticipantInfo(String name, String teamName, String sectionName) { + private ParticipantInfo(String name, Team team, Section section) { this.name = name; - this.teamName = teamName; - this.sectionName = sectionName; + this.team = team; + this.section = section; } public String getName() { return name; } + public Team getTeam() { + return team; + } + + public Section getSection() { + return section; + } + public String getTeamName() { - return teamName; + return team.getName(); } public String getSectionName() { - return sectionName; + return section.getName(); } } } diff --git a/src/main/java/teammates/storage/sqlentity/FeedbackQuestion.java b/src/main/java/teammates/storage/sqlentity/FeedbackQuestion.java index 19f81cf0678..d5747a28dfd 100644 --- a/src/main/java/teammates/storage/sqlentity/FeedbackQuestion.java +++ b/src/main/java/teammates/storage/sqlentity/FeedbackQuestion.java @@ -239,6 +239,10 @@ public FeedbackSession getFeedbackSession() { return feedbackSession; } + public String getFeedbackSessionName() { + return feedbackSession.getName(); + } + public void setFeedbackSession(FeedbackSession feedbackSession) { this.feedbackSession = feedbackSession; } @@ -328,6 +332,10 @@ public void setUpdatedAt(Instant updatedAt) { this.updatedAt = updatedAt; } + public Course getCourse() { + return this.feedbackSession.getCourse(); + } + public String getCourseId() { return this.feedbackSession.getCourse().getId(); } diff --git a/src/main/java/teammates/storage/sqlentity/FeedbackResponseComment.java b/src/main/java/teammates/storage/sqlentity/FeedbackResponseComment.java index 311c836ae8f..be969ae6eb2 100644 --- a/src/main/java/teammates/storage/sqlentity/FeedbackResponseComment.java +++ b/src/main/java/teammates/storage/sqlentity/FeedbackResponseComment.java @@ -110,6 +110,10 @@ public FeedbackResponse getFeedbackResponse() { return feedbackResponse; } + public FeedbackQuestion getFeedbackQuestion() { + return getFeedbackResponse().getFeedbackQuestion(); + } + public void setFeedbackResponse(FeedbackResponse feedbackResponse) { this.feedbackResponse = feedbackResponse; } From da63d4a373c7f58106bb4d64a83a5e4e2aceef0d Mon Sep 17 00:00:00 2001 From: Xenos F Date: Mon, 12 Feb 2024 18:01:52 +0800 Subject: [PATCH 04/42] Migrate buildCompleteGiverRecipientMap --- .../sqllogic/core/FeedbackQuestionsLogic.java | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/src/main/java/teammates/sqllogic/core/FeedbackQuestionsLogic.java b/src/main/java/teammates/sqllogic/core/FeedbackQuestionsLogic.java index 208f0efab4b..bf700b31b86 100644 --- a/src/main/java/teammates/sqllogic/core/FeedbackQuestionsLogic.java +++ b/src/main/java/teammates/sqllogic/core/FeedbackQuestionsLogic.java @@ -12,9 +12,14 @@ import javax.annotation.Nullable; +import teammates.common.datatransfer.CourseRoster; import teammates.common.datatransfer.FeedbackParticipantType; import teammates.common.datatransfer.FeedbackQuestionRecipient; import teammates.common.datatransfer.SqlCourseRoster; +import teammates.common.datatransfer.attributes.FeedbackQuestionAttributes; +import teammates.common.datatransfer.attributes.FeedbackSessionAttributes; +import teammates.common.datatransfer.attributes.InstructorAttributes; +import teammates.common.datatransfer.attributes.StudentAttributes; import teammates.common.datatransfer.questions.FeedbackMcqQuestionDetails; import teammates.common.datatransfer.questions.FeedbackMsqQuestionDetails; import teammates.common.datatransfer.questions.FeedbackQuestionDetails; @@ -647,4 +652,100 @@ public List getFeedbackQuestionForCourseWithType( .collect(Collectors.toList()); } + + /** + * Builds a complete giver to recipient map for a {@code relatedQuestion}. + * + * @param relatedQuestion The question to be considered + * @param courseRoster the roster in the course + * @return a map from giver to recipient for the question. + */ + public Map> buildCompleteGiverRecipientMap( + FeedbackQuestion relatedQuestion, SqlCourseRoster courseRoster) { + Map> completeGiverRecipientMap = new HashMap<>(); + + List possibleGiverEmails = getPossibleGivers(relatedQuestion, courseRoster); + for (String possibleGiverEmail : possibleGiverEmails) { + switch (relatedQuestion.getGiverType()) { + case STUDENTS: + Student studentGiver = courseRoster.getStudentForEmail(possibleGiverEmail); + completeGiverRecipientMap + .computeIfAbsent(possibleGiverEmail, key -> new HashSet<>()) + .addAll(getRecipientsOfQuestion( + relatedQuestion, null, studentGiver, courseRoster).keySet()); + break; + case TEAMS: + Student oneTeamMember = + courseRoster.getTeamToMembersTable().get(possibleGiverEmail).iterator().next(); + completeGiverRecipientMap + .computeIfAbsent(possibleGiverEmail, key -> new HashSet<>()) + .addAll(getRecipientsOfQuestion( + relatedQuestion, null, oneTeamMember, courseRoster).keySet()); + break; + case INSTRUCTORS: + case SELF: + Instructor instructorGiver = courseRoster.getInstructorForEmail(possibleGiverEmail); + + // only happens when a session creator quits their course + if (instructorGiver == null) { + instructorGiver = new Instructor(relatedQuestion.getCourse(), USER_NAME_FOR_SELF, possibleGiverEmail, false, USER_NAME_FOR_SELF, null, null); + } + + completeGiverRecipientMap + .computeIfAbsent(possibleGiverEmail, key -> new HashSet<>()) + .addAll(getRecipientsOfQuestion( + relatedQuestion, instructorGiver, null, courseRoster).keySet()); + break; + default: + log.severe("Invalid giver type specified"); + break; + } + } + + return completeGiverRecipientMap; + } + + + /** + * Gets possible giver identifiers for a feedback question. + * + * @param fq the feedback question + * @param courseRoster roster of all students and instructors + * @return a list of giver identifier + */ + private List getPossibleGivers( + FeedbackQuestion fq, SqlCourseRoster courseRoster) { + FeedbackParticipantType giverType = fq.getGiverType(); + List possibleGivers = new ArrayList<>(); + + switch (giverType) { + case STUDENTS: + possibleGivers = courseRoster.getStudents() + .stream() + .map(Student::getEmail) + .collect(Collectors.toList()); + break; + case INSTRUCTORS: + possibleGivers = courseRoster.getInstructors() + .stream() + .map(Instructor::getEmail) + .collect(Collectors.toList()); + break; + case TEAMS: + possibleGivers = new ArrayList<>(courseRoster.getTeamToMembersTable().keySet()); + break; + case SELF: + FeedbackSession feedbackSession = + feedbackSessionsLogic.getFeedbackSession(fq.getFeedbackSessionName(), fq.getCourseId()); + possibleGivers = Collections.singletonList(feedbackSession.getCreatorEmail()); + break; + default: + log.severe("Invalid giver type specified"); + break; + } + + return possibleGivers; + } + + } From cc8ff7acbd40e99874399d849f27476944fd16d0 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Fri, 2 Feb 2024 00:26:09 +0800 Subject: [PATCH 05/42] Migrate checkSpecificAccessControl --- .../ui/webapi/GetSessionResultsAction.java | 89 +++++++++++++------ 1 file changed, 62 insertions(+), 27 deletions(-) diff --git a/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java b/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java index 946dd44eba5..17d409c8175 100644 --- a/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java +++ b/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java @@ -6,13 +6,16 @@ import teammates.common.datatransfer.attributes.InstructorAttributes; import teammates.common.datatransfer.attributes.StudentAttributes; import teammates.common.util.Const; +import teammates.storage.sqlentity.FeedbackSession; +import teammates.storage.sqlentity.Instructor; +import teammates.storage.sqlentity.Student; import teammates.ui.output.SessionResultsData; import teammates.ui.request.Intent; /** * Gets feedback session results including statistics where necessary. */ -class GetSessionResultsAction extends Action { +public class GetSessionResultsAction extends Action { @Override AuthType getMinAuthLevel() { @@ -23,34 +26,66 @@ AuthType getMinAuthLevel() { void checkSpecificAccessControl() throws UnauthorizedAccessException { String courseId = getNonNullRequestParamValue(Const.ParamsNames.COURSE_ID); String feedbackSessionName = getNonNullRequestParamValue(Const.ParamsNames.FEEDBACK_SESSION_NAME); - - FeedbackSessionAttributes fs = getNonNullFeedbackSession(feedbackSessionName, courseId); Intent intent = Intent.valueOf(getNonNullRequestParamValue(Const.ParamsNames.INTENT)); - switch (intent) { - case FULL_DETAIL: - gateKeeper.verifyLoggedInUserPrivileges(userInfo); - InstructorAttributes instructor = logic.getInstructorForGoogleId(courseId, userInfo.getId()); - gateKeeper.verifyAccessible(instructor, fs); - break; - case INSTRUCTOR_RESULT: - instructor = getPossiblyUnregisteredInstructor(courseId); - gateKeeper.verifyAccessible(instructor, fs); - if (!fs.isPublished()) { - throw new UnauthorizedAccessException("This feedback session is not yet published.", true); - } - break; - case STUDENT_RESULT: - StudentAttributes student = getPossiblyUnregisteredStudent(courseId); - gateKeeper.verifyAccessible(student, fs); - if (!fs.isPublished()) { - throw new UnauthorizedAccessException("This feedback session is not yet published.", true); + + if (isCourseMigrated(courseId)) { + FeedbackSession feedbackSession = getNonNullSqlFeedbackSession(feedbackSessionName, courseId); + + switch (intent) { + case FULL_DETAIL: + gateKeeper.verifyLoggedInUserPrivileges(userInfo); + Instructor instructor = sqlLogic.getInstructorByGoogleId(courseId, userInfo.getId()); + gateKeeper.verifyAccessible(instructor, feedbackSession); + break; + case INSTRUCTOR_RESULT: + instructor = getPossiblyUnregisteredSqlInstructor(courseId); + gateKeeper.verifyAccessible(instructor, feedbackSession); + if (!feedbackSession.isPublished()) { + throw new UnauthorizedAccessException("This feedback session is not yet published.", true); + } + break; + case STUDENT_RESULT: + Student student = getPossiblyUnregisteredSqlStudent(courseId); + gateKeeper.verifyAccessible(student, feedbackSession); + if (!feedbackSession.isPublished()) { + throw new UnauthorizedAccessException("This feedback session is not yet published.", true); + } + break; + case INSTRUCTOR_SUBMISSION: + case STUDENT_SUBMISSION: + throw new InvalidHttpParameterException("Invalid intent for this action"); + default: + throw new InvalidHttpParameterException("Unknown intent " + intent); + } + } else { + FeedbackSessionAttributes feedbackSession = getNonNullFeedbackSession(feedbackSessionName, courseId); + + switch (intent) { + case FULL_DETAIL: + gateKeeper.verifyLoggedInUserPrivileges(userInfo); + InstructorAttributes instructor = logic.getInstructorForGoogleId(courseId, userInfo.getId()); + gateKeeper.verifyAccessible(instructor, feedbackSession); + break; + case INSTRUCTOR_RESULT: + instructor = getPossiblyUnregisteredInstructor(courseId); + gateKeeper.verifyAccessible(instructor, feedbackSession); + if (!feedbackSession.isPublished()) { + throw new UnauthorizedAccessException("This feedback session is not yet published.", true); + } + break; + case STUDENT_RESULT: + StudentAttributes student = getPossiblyUnregisteredStudent(courseId); + gateKeeper.verifyAccessible(student, feedbackSession); + if (!feedbackSession.isPublished()) { + throw new UnauthorizedAccessException("This feedback session is not yet published.", true); + } + break; + case INSTRUCTOR_SUBMISSION: + case STUDENT_SUBMISSION: + throw new InvalidHttpParameterException("Invalid intent for this action"); + default: + throw new InvalidHttpParameterException("Unknown intent " + intent); } - break; - case INSTRUCTOR_SUBMISSION: - case STUDENT_SUBMISSION: - throw new InvalidHttpParameterException("Invalid intent for this action"); - default: - throw new InvalidHttpParameterException("Unknown intent " + intent); } } From a046670a2305507f1a98266464a4c0eef9be9ea9 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Sun, 4 Feb 2024 15:17:19 +0800 Subject: [PATCH 06/42] Add default team instance for instructor --- src/main/java/teammates/storage/sqlentity/Instructor.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/teammates/storage/sqlentity/Instructor.java b/src/main/java/teammates/storage/sqlentity/Instructor.java index 6848d80ac4b..e9d7ea56b98 100644 --- a/src/main/java/teammates/storage/sqlentity/Instructor.java +++ b/src/main/java/teammates/storage/sqlentity/Instructor.java @@ -39,6 +39,8 @@ public class Instructor extends User { @Convert(converter = InstructorPrivilegesConverter.class) private InstructorPrivileges privileges; + private static Team userTeam = new Team(null, Const.USER_TEAM_FOR_INSTRUCTOR); + protected Instructor() { // required by Hibernate } @@ -50,6 +52,7 @@ public Instructor(Course course, String name, String email, boolean isDisplayedT this.setDisplayName(displayName); this.setRole(role); this.setPrivileges(privileges); + this.setTeam(userTeam); } public boolean isDisplayedToStudents() { From 9b4945b2ba8532996e04bf48e299ef78cf4531cb Mon Sep 17 00:00:00 2001 From: Xenos F Date: Sun, 4 Feb 2024 15:23:51 +0800 Subject: [PATCH 07/42] Migrate session results data logic --- .../java/teammates/ui/output/SessionResultsData.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/main/java/teammates/ui/output/SessionResultsData.java b/src/main/java/teammates/ui/output/SessionResultsData.java index 81045916899..61aa7c90022 100644 --- a/src/main/java/teammates/ui/output/SessionResultsData.java +++ b/src/main/java/teammates/ui/output/SessionResultsData.java @@ -30,7 +30,6 @@ import teammates.storage.sqlentity.Instructor; import teammates.storage.sqlentity.Section; import teammates.storage.sqlentity.Student; -import teammates.ui.output.SessionResultsData.CommentOutput; /** * API output format for session results, including statistics. @@ -177,7 +176,7 @@ public static SessionResultsData initForStudent(SqlSessionResultsBundle bundle, if (questionDetails.isIndividualResponsesShownToStudents()) { for (FeedbackResponse response : responses) { - boolean isUserInstructor = Const.USER_TEAM_FOR_INSTRUCTOR.equals(student.getTeam()); + boolean isUserInstructor = Const.USER_TEAM_FOR_INSTRUCTOR.equals(student.getTeamName()); boolean isUserGiver = student.getEmail().equals(response.getGiver()) && (isUserInstructor && question.getGiverType() == FeedbackParticipantType.INSTRUCTORS @@ -286,14 +285,14 @@ private static ResponseOutput buildSingleResponseForStudent( private static ResponseOutput buildSingleResponseForStudent( FeedbackResponse response, SqlSessionResultsBundle bundle, Student student) { FeedbackQuestion question = response.getFeedbackQuestion(); - boolean isUserInstructor = Const.USER_TEAM_FOR_INSTRUCTOR.equals(student.getTeam()); + boolean isUserInstructor = Const.USER_TEAM_FOR_INSTRUCTOR.equals(student.getTeamName()); // process giver boolean isUserGiver = student.getEmail().equals(response.getGiver()) && (isUserInstructor && question.getGiverType() == FeedbackParticipantType.INSTRUCTORS || !isUserInstructor && question.getGiverType() != FeedbackParticipantType.INSTRUCTORS); boolean isUserTeamGiver = question.getGiverType() == FeedbackParticipantType.TEAMS - && student.getTeam().equals(response.getGiver()); + && student.getTeamName().equals(response.getGiver()); String giverName; String giverTeam = ""; if (isUserTeamGiver) { @@ -313,7 +312,7 @@ private static ResponseOutput buildSingleResponseForStudent( || !isUserInstructor && question.getRecipientType() != FeedbackParticipantType.INSTRUCTORS); boolean isUserTeamRecipient = (question.getRecipientType() == FeedbackParticipantType.TEAMS || question.getRecipientType() == FeedbackParticipantType.TEAMS_IN_SAME_SECTION) - && student.getTeam().equals(response.getRecipient()); + && student.getTeamName().equals(response.getRecipient()); String recipientName; String recipientTeam = ""; if (isUserRecipient) { @@ -379,7 +378,6 @@ private static List buildResponsesForInstructor( return output; } - private static ResponseOutput buildSingleResponseForInstructor( FeedbackResponseAttributes response, SessionResultsBundle bundle, boolean isMissingResponse) { // process giver From fb5a808f380c136e1521ef239a4b747523320762 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Mon, 12 Feb 2024 18:15:13 +0800 Subject: [PATCH 08/42] Use default team entity for instructor instead of const --- .../java/teammates/common/datatransfer/SqlCourseRoster.java | 2 +- src/main/java/teammates/common/util/Const.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/teammates/common/datatransfer/SqlCourseRoster.java b/src/main/java/teammates/common/datatransfer/SqlCourseRoster.java index 83b8021cfb2..dd43e5876e5 100644 --- a/src/main/java/teammates/common/datatransfer/SqlCourseRoster.java +++ b/src/main/java/teammates/common/datatransfer/SqlCourseRoster.java @@ -144,7 +144,7 @@ public ParticipantInfo getInfoForIdentifier(String identifier) { Instructor instructor = getInstructorForEmail(identifier); name = instructor.getName(); - team = Const.USER_TEAM_ENTITY_FOR_INSTRUCTOR; + team = instructor.getTeam(); section = Const.DEFAULT_SECTION_ENTITY; } else if (isTeam) { Student teamMember = getTeamToMembersTable().get(identifier).iterator().next(); diff --git a/src/main/java/teammates/common/util/Const.java b/src/main/java/teammates/common/util/Const.java index c999a9d42c7..bcfe84f6d5c 100644 --- a/src/main/java/teammates/common/util/Const.java +++ b/src/main/java/teammates/common/util/Const.java @@ -20,7 +20,6 @@ public final class Const { public static final String USER_NOBODY_TEXT = "-"; public static final String USER_TEAM_FOR_INSTRUCTOR = "Instructors"; - public static final Team USER_TEAM_ENTITY_FOR_INSTRUCTOR = new Team(null, USER_TEAM_FOR_INSTRUCTOR); public static final String DEFAULT_DISPLAY_NAME_FOR_INSTRUCTOR = "Instructor"; From cd64a4c678f7a8813a11688f89144da093a56385 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Wed, 14 Feb 2024 14:22:43 +0800 Subject: [PATCH 09/42] Migrate non-db logic --- .../questions/FeedbackQuestionDetails.java | 11 + .../java/teammates/sqllogic/api/Logic.java | 37 ++ .../core/FeedbackResponseCommentsLogic.java | 201 ++++++ .../sqllogic/core/FeedbackResponsesLogic.java | 578 ++++++++++++++++++ .../storage/sqlentity/FeedbackResponse.java | 8 + .../sqlentity/FeedbackResponseComment.java | 7 + .../ui/webapi/GetSessionResultsAction.java | 119 ++-- 7 files changed, 924 insertions(+), 37 deletions(-) diff --git a/src/main/java/teammates/common/datatransfer/questions/FeedbackQuestionDetails.java b/src/main/java/teammates/common/datatransfer/questions/FeedbackQuestionDetails.java index 573193dfd0b..c763c06902c 100644 --- a/src/main/java/teammates/common/datatransfer/questions/FeedbackQuestionDetails.java +++ b/src/main/java/teammates/common/datatransfer/questions/FeedbackQuestionDetails.java @@ -125,6 +125,17 @@ public boolean shouldGenerateMissingResponses(FeedbackQuestionAttributes questio && question.getRecipientType() != FeedbackParticipantType.TEAMS_EXCLUDING_SELF; } + /** + * Checks whether missing responses should be generated. + */ + public boolean shouldGenerateMissingResponses(FeedbackQuestion question) { + // generate combinations against all students/teams are meaningless + return question.getRecipientType() != FeedbackParticipantType.STUDENTS + && question.getRecipientType() != FeedbackParticipantType.STUDENTS_EXCLUDING_SELF + && question.getRecipientType() != FeedbackParticipantType.TEAMS + && question.getRecipientType() != FeedbackParticipantType.TEAMS_EXCLUDING_SELF; + } + @Override public boolean equals(Object obj) { if (this == obj) { diff --git a/src/main/java/teammates/sqllogic/api/Logic.java b/src/main/java/teammates/sqllogic/api/Logic.java index 948dad06746..f21c0078a41 100644 --- a/src/main/java/teammates/sqllogic/api/Logic.java +++ b/src/main/java/teammates/sqllogic/api/Logic.java @@ -8,10 +8,13 @@ import javax.annotation.Nullable; +import teammates.common.datatransfer.DataBundle; import teammates.common.datatransfer.FeedbackQuestionRecipient; +import teammates.common.datatransfer.FeedbackResultFetchType; import teammates.common.datatransfer.NotificationStyle; import teammates.common.datatransfer.NotificationTargetUser; import teammates.common.datatransfer.SqlDataBundle; +import teammates.common.datatransfer.SqlSessionResultsBundle; import teammates.common.exception.EnrollException; import teammates.common.exception.EntityAlreadyExistsException; import teammates.common.exception.EntityDoesNotExistException; @@ -1092,6 +1095,40 @@ public List getFeedbackQuestionsForInstructors( return feedbackQuestionsLogic.getFeedbackQuestionsForInstructors(feedbackSession, instructorEmail); } + + /** + * Gets the session result for a feedback session. + * + * @see FeedbackResponsesLogic#getSessionResultsForCourse( + * FeedbackSession, String, String, String, String, FeedbackResultFetchType) + */ + public SqlSessionResultsBundle getSessionResultsForCourse( + FeedbackSession feedbackSession, String courseId, String userEmail, + @Nullable UUID questionId, @Nullable String section, @Nullable FeedbackResultFetchType fetchType) { + assert feedbackSession != null; + assert courseId != null; + assert userEmail != null; + + return feedbackResponsesLogic.getSessionResultsForCourse( + feedbackSession, courseId, userEmail, questionId, section, fetchType); + } + + /** + * Gets the session result for a feedback session for the given user. + * + * @see FeedbackResponsesLogic#getSessionResultsForUser(FeedbackSession, String, String, boolean, String) + */ + public SqlSessionResultsBundle getSessionResultsForUser( + FeedbackSession feedbackSession, String courseId, String userEmail, boolean isInstructor, + @Nullable UUID questionId) { + assert feedbackSession != null; + assert courseId != null; + assert userEmail != null; + + return feedbackResponsesLogic.getSessionResultsForUser( + feedbackSession, courseId, userEmail, isInstructor, questionId); + } + /** * Persists the given data bundle to the database. */ diff --git a/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java b/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java index c6aabfa0153..89808f6631e 100644 --- a/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java +++ b/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java @@ -1,14 +1,21 @@ package teammates.sqllogic.core; import java.util.List; +import java.util.Set; import java.util.UUID; +import javax.annotation.Nullable; + +import teammates.common.datatransfer.FeedbackParticipantType; +import teammates.common.datatransfer.SqlCourseRoster; import teammates.common.exception.EntityAlreadyExistsException; import teammates.common.exception.EntityDoesNotExistException; import teammates.common.exception.InvalidParametersException; import teammates.storage.sqlapi.FeedbackResponseCommentsDb; +import teammates.storage.sqlentity.FeedbackQuestion; import teammates.storage.sqlentity.FeedbackResponse; import teammates.storage.sqlentity.FeedbackResponseComment; +import teammates.storage.sqlentity.Student; import teammates.ui.request.FeedbackResponseCommentUpdateRequest; /** @@ -128,4 +135,198 @@ public void updateFeedbackResponseCommentsForResponse(FeedbackResponse response) } } + + /** + * Gets all feedback response comments for session in a section. + * + * @param courseId the course ID of the feedback session + * @param feedbackSessionName the feedback session name + * @param section if null, will retrieve all comments in the session + * @return a list of feedback response comments + */ + public List getFeedbackResponseCommentForSessionInSection( + String courseId, String feedbackSessionName, @Nullable String section) { + if (section == null) { + return frcDb.getFeedbackResponseCommentsForSession(courseId, feedbackSessionName); + } + return frcDb.getFeedbackResponseCommentsForSessionInSection(courseId, feedbackSessionName, section); + } + + /** + * Gets all feedback response comments for a question in a section. + * + * @param questionId the ID of the question + * @param section if null, will retrieve all comments for the question + * @return a list of feedback response comments + */ + public List getFeedbackResponseCommentForQuestionInSection( + UUID questionId, @Nullable String section) { + if (section == null) { + return frcDb.getFeedbackResponseCommentsForQuestion(questionId); + } + return frcDb.getFeedbackResponseCommentsForQuestionInSection(questionId, section); + } + + + /** + * Verifies whether the comment is visible to certain user. + * @return true/false + */ + public boolean isResponseCommentVisibleForUser(String userEmail, boolean isInstructor, + Student student, Set studentsEmailInTeam, FeedbackResponse response, + FeedbackQuestion relatedQuestion, FeedbackResponseComment relatedComment) { + + if (response == null || relatedQuestion == null) { + return false; + } + + boolean isVisibilityFollowingFeedbackQuestion = relatedComment.getIsVisibilityFollowingFeedbackQuestion(); + boolean isVisibleToGiver = isVisibilityFollowingFeedbackQuestion + || relatedComment.isVisibleTo(FeedbackParticipantType.GIVER); + + boolean isVisibleToUser = isVisibleToUser(userEmail, response, relatedQuestion, relatedComment, + isVisibleToGiver, isInstructor, !isInstructor); + + boolean isVisibleToUserTeam = isVisibleToUserTeam(student, studentsEmailInTeam, response, + relatedQuestion, relatedComment, !isInstructor); + + return isVisibleToUser || isVisibleToUserTeam; + } + + private boolean isVisibleToUserTeam(Student student, Set studentsEmailInTeam, + FeedbackResponse response, FeedbackQuestion relatedQuestion, + FeedbackResponseComment relatedComment, boolean isUserStudent) { + + boolean isUserInResponseRecipientTeamAndRelatedResponseCommentVisibleToRecipients = + isUserStudent + && relatedQuestion.getRecipientType() == FeedbackParticipantType.TEAMS + && isResponseCommentVisibleTo(relatedQuestion, relatedComment, + FeedbackParticipantType.RECEIVER) + && response.getRecipient().equals(student.getTeamName()); + + boolean isUserInResponseGiverTeamAndRelatedResponseCommentVisibleToGiversTeamMembers = + (relatedQuestion.getGiverType() == FeedbackParticipantType.TEAMS + || isResponseCommentVisibleTo(relatedQuestion, relatedComment, + FeedbackParticipantType.OWN_TEAM_MEMBERS)) + && (studentsEmailInTeam.contains(response.getGiver()) + || isUserStudent && student.getTeamName().equals(response.getGiver())); + + boolean isUserInResponseRecipientTeamAndRelatedResponseCommentVisibleToRecipientsTeamMembers = + isResponseCommentVisibleTo(relatedQuestion, relatedComment, + FeedbackParticipantType.RECEIVER_TEAM_MEMBERS) + && studentsEmailInTeam.contains(response.getRecipient()); + + return isUserInResponseRecipientTeamAndRelatedResponseCommentVisibleToRecipients + || isUserInResponseGiverTeamAndRelatedResponseCommentVisibleToGiversTeamMembers + || isUserInResponseRecipientTeamAndRelatedResponseCommentVisibleToRecipientsTeamMembers; + } + + private boolean isVisibleToUser(String userEmail, FeedbackResponse response, + FeedbackQuestion relatedQuestion, FeedbackResponseComment relatedComment, + boolean isVisibleToGiver, boolean isUserInstructor, boolean isUserStudent) { + + boolean isUserInstructorAndRelatedResponseCommentVisibleToInstructors = + isUserInstructor && isResponseCommentVisibleTo(relatedQuestion, relatedComment, + FeedbackParticipantType.INSTRUCTORS); + + boolean isUserResponseRecipientAndRelatedResponseCommentVisibleToRecipients = + response.getRecipient().equals(userEmail) && isResponseCommentVisibleTo(relatedQuestion, + relatedComment, FeedbackParticipantType.RECEIVER); + + boolean isUserResponseGiverAndRelatedResponseCommentVisibleToGivers = + response.getGiver().equals(userEmail) && isVisibleToGiver; + + boolean isUserRelatedResponseCommentGiver = relatedComment.getGiver().equals(userEmail); + + boolean isUserStudentAndRelatedResponseCommentVisibleToStudents = + isUserStudent && isResponseCommentVisibleTo(relatedQuestion, + relatedComment, FeedbackParticipantType.STUDENTS); + + return isUserInstructorAndRelatedResponseCommentVisibleToInstructors + || isUserResponseRecipientAndRelatedResponseCommentVisibleToRecipients + || isUserResponseGiverAndRelatedResponseCommentVisibleToGivers + || isUserRelatedResponseCommentGiver + || isUserStudentAndRelatedResponseCommentVisibleToStudents; + } + + private boolean isResponseCommentVisibleTo(FeedbackQuestion relatedQuestion, + FeedbackResponseComment relatedComment, + FeedbackParticipantType viewerType) { + boolean isVisibilityFollowingFeedbackQuestion = relatedComment.getIsVisibilityFollowingFeedbackQuestion(); + return isVisibilityFollowingFeedbackQuestion + ? relatedQuestion.isResponseVisibleTo(viewerType) + : relatedComment.isVisibleTo(viewerType); + } + + /** + * Returns true if the comment's giver name is visible to certain user. + */ + public boolean isNameVisibleToUser(FeedbackResponseComment comment, FeedbackResponse response, + String userEmail, SqlCourseRoster roster) { + List showNameTo = comment.getShowGiverNameTo(); + //in the old ver, name is always visible + if (showNameTo == null || comment.getIsVisibilityFollowingFeedbackQuestion()) { + return true; + } + + //comment giver can always see + if (userEmail.equals(comment.getGiver())) { + return true; + } + + return isFeedbackParticipantNameVisibleToUser(response, userEmail, roster, showNameTo); + } + + private boolean isFeedbackParticipantNameVisibleToUser(FeedbackResponse response, + String userEmail, SqlCourseRoster roster, List showNameTo) { + String responseGiverTeam = "giverTeam"; + if (roster.getStudentForEmail(response.getGiver()) != null) { + responseGiverTeam = roster.getStudentForEmail(response.getGiver()).getTeamName(); + } + String responseRecipientTeam = "recipientTeam"; + if (roster.getStudentForEmail(response.getRecipient()) != null) { + responseRecipientTeam = roster.getStudentForEmail(response.getRecipient()).getTeamName(); + } + String currentUserTeam = "currentUserTeam"; + if (roster.getStudentForEmail(userEmail) != null) { + currentUserTeam = roster.getStudentForEmail(userEmail).getTeamName(); + } + for (FeedbackParticipantType type : showNameTo) { + switch (type) { + case INSTRUCTORS: + if (roster.getInstructorForEmail(userEmail) != null) { + return true; + } + break; + case OWN_TEAM_MEMBERS: + if (responseGiverTeam.equals(currentUserTeam)) { + return true; + } + break; + case RECEIVER: + if (userEmail.equals(response.getRecipient())) { + return true; + } + break; + case RECEIVER_TEAM_MEMBERS: + if (responseRecipientTeam.equals(currentUserTeam)) { + return true; + } + break; + case STUDENTS: + if (roster.getStudentForEmail(userEmail) != null) { + return true; + } + break; + case GIVER: + if (userEmail.equals(response.getGiver())) { + return true; + } + break; + default: + break; + } + } + return false; + } } diff --git a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java index 7e3cb8d13e4..52ab88b1a1f 100644 --- a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java +++ b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java @@ -1,23 +1,34 @@ package teammates.sqllogic.core; import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.UUID; import javax.annotation.Nullable; import teammates.common.datatransfer.FeedbackParticipantType; +import teammates.common.datatransfer.FeedbackResultFetchType; import teammates.common.datatransfer.SqlCourseRoster; +import teammates.common.datatransfer.SqlSessionResultsBundle; import teammates.common.datatransfer.questions.FeedbackQuestionType; import teammates.common.datatransfer.questions.FeedbackRankRecipientsResponseDetails; +import teammates.common.datatransfer.questions.FeedbackTextResponseDetails; import teammates.common.exception.EntityAlreadyExistsException; import teammates.common.exception.EntityDoesNotExistException; import teammates.common.exception.InvalidParametersException; +import teammates.common.util.Const; +import teammates.common.util.RequestTracer; import teammates.storage.sqlapi.FeedbackResponsesDb; import teammates.storage.sqlentity.Course; import teammates.storage.sqlentity.FeedbackQuestion; import teammates.storage.sqlentity.FeedbackResponse; +import teammates.storage.sqlentity.FeedbackResponseComment; +import teammates.storage.sqlentity.FeedbackSession; import teammates.storage.sqlentity.Instructor; import teammates.storage.sqlentity.Section; import teammates.storage.sqlentity.Student; @@ -451,5 +462,572 @@ public void updateFeedbackResponsesForChangingSection(Course course, String newE frcLogic.updateFeedbackResponseCommentsForResponse(response); } } + + private List getQuestionsForSession( + FeedbackSession feedbackSession, String courseId, @Nullable UUID questionId) { + if (questionId == null) { + return fqLogic.getFeedbackQuestionsForSession(feedbackSession); + } + FeedbackQuestion fq = fqLogic.getFeedbackQuestion(questionId); + return fq == null ? Collections.emptyList() : Collections.singletonList(fq); + } + + private SqlSessionResultsBundle buildResultsBundle( + boolean isCourseWide, FeedbackSession feedbackSession, String courseId, String section, UUID questionId, + boolean isInstructor, String userEmail, Instructor instructor, Student student, + SqlCourseRoster roster, List allQuestions, + List allResponses) { + + // load comment(s) + List allComments; + if (questionId == null) { + allComments = frcLogic.getFeedbackResponseCommentForSessionInSection(courseId, feedbackSession.getName(), section); + } else { + allComments = frcLogic.getFeedbackResponseCommentForQuestionInSection(questionId, section); + } + RequestTracer.checkRemainingTime(); + + // related questions, responses, and comment + List relatedQuestions = new ArrayList<>(); + List relatedResponses = new ArrayList<>(); + Map> relatedCommentsMap = new HashMap<>(); + if (isCourseWide) { + // all questions are related questions when viewing course-wide result + for (FeedbackQuestion qn : allQuestions) { + relatedQuestions.add(qn); + } + } + + Set studentsEmailInTeam = new HashSet<>(); + if (student != null) { + for (Student studentInTeam + : roster.getTeamToMembersTable().getOrDefault(student.getTeam(), Collections.emptyList())) { + studentsEmailInTeam.add(studentInTeam.getEmail()); + } + } + + // visibility table for each response and comment + Map responseGiverVisibilityTable = new HashMap<>(); + Map responseRecipientVisibilityTable = new HashMap<>(); + Map commentVisibilityTable = new HashMap<>(); + + // build response + for (FeedbackResponse response : allResponses) { + FeedbackQuestion correspondingQuestion = response.getFeedbackQuestion(); + if (correspondingQuestion == null) { + // orphan response without corresponding question, ignore it + continue; + } + // check visibility of response + boolean isVisibleResponse = isResponseVisibleForUser( + userEmail, isInstructor, student, studentsEmailInTeam, response, correspondingQuestion, instructor); + if (!isVisibleResponse) { + continue; + } + + // if there are viewable responses, the corresponding question becomes related + relatedQuestions.add(response.getFeedbackQuestion()); + relatedResponses.add(response); + + // generate giver/recipient name visibility table + responseGiverVisibilityTable.put(response, + isNameVisibleToUser(correspondingQuestion, response, userEmail, isInstructor, true, roster)); + responseRecipientVisibilityTable.put(response, + isNameVisibleToUser(correspondingQuestion, response, userEmail, isInstructor, false, roster)); + } + RequestTracer.checkRemainingTime(); + + // build comment + for (FeedbackResponseComment frc : allComments) { + FeedbackResponse relatedResponse = frc.getFeedbackResponse(); + FeedbackQuestion relatedQuestion = frc.getFeedbackQuestion(); + // the comment needs to be relevant to the question and response + if (relatedQuestion == null || relatedResponse == null) { + continue; + } + // check visibility of comment + boolean isVisibleResponseComment = frcLogic.isResponseCommentVisibleForUser( + userEmail, isInstructor, student, studentsEmailInTeam, relatedResponse, relatedQuestion, frc); + if (!isVisibleResponseComment) { + continue; + } + + relatedCommentsMap.computeIfAbsent(relatedResponse, key -> new ArrayList<>()).add(frc); + // generate comment giver name visibility table + commentVisibilityTable.put(frc.getId(), frcLogic.isNameVisibleToUser(frc, relatedResponse, userEmail, roster)); + } + RequestTracer.checkRemainingTime(); + + List existingResponses = new ArrayList<>(relatedResponses); + List missingResponses = Collections.emptyList(); + if (isCourseWide) { + missingResponses = buildMissingResponses( + courseId, feedbackSession, instructor, responseGiverVisibilityTable, + responseRecipientVisibilityTable, relatedQuestions, existingResponses, roster, section); + } + RequestTracer.checkRemainingTime(); + + return new SqlSessionResultsBundle(relatedQuestions, existingResponses, missingResponses, + responseGiverVisibilityTable, responseRecipientVisibilityTable, relatedCommentsMap, + commentVisibilityTable, roster); + } + + /** + * Gets the session result for a feedback session. + * + * @param feedbackSession the feedback session + * @param courseId the ID of the course + * @param instructorEmail the instructor viewing the feedback session + * @param questionId if not null, will only return partial bundle for the question + * @param section if not null, will only return partial bundle for the section + * @param fetchType if not null, will fetch responses by giver, receiver sections, or both + * @return the session result bundle + */ + public SqlSessionResultsBundle getSessionResultsForCourse( + FeedbackSession feedbackSession, String courseId, String instructorEmail, + @Nullable UUID questionId, @Nullable String section, @Nullable FeedbackResultFetchType fetchType) { + + SqlCourseRoster roster = new SqlCourseRoster( + usersLogic.getStudentsForCourse(courseId), + usersLogic.getInstructorsForCourse(courseId)); + + // load question(s) + List allQuestions = getQuestionsForSession(feedbackSession, courseId, questionId); + RequestTracer.checkRemainingTime(); + + // load response(s) + List allResponses; + // load all response for instructors and passively filter them later + if (questionId == null) { + allResponses = getFeedbackResponsesForSessionInSection(feedbackSession, courseId, section, fetchType); + } else { + allResponses = getFeedbackResponsesForQuestionInSection(questionId, section, fetchType); + } + RequestTracer.checkRemainingTime(); + + // consider the current viewing user + Instructor instructor = usersLogic.getInstructorForEmail(courseId, instructorEmail); + + return buildResultsBundle(true, feedbackSession, courseId, section, questionId, true, instructorEmail, + instructor, null, roster, allQuestions, allResponses); + } + + /** + * Gets the session result for a feedback session for the given user. + * + * @param feedbackSession the feedback session + * @param courseId the ID of the course + * @param userEmail the user viewing the feedback session + * @param isInstructor true if the user is an instructor + * @param questionId if not null, will only return partial bundle for the question + * @return the session result bundle + */ + public SqlSessionResultsBundle getSessionResultsForUser( + FeedbackSession feedbackSession, String courseId, String userEmail, boolean isInstructor, + @Nullable UUID questionId) { + SqlCourseRoster roster = new SqlCourseRoster( + usersLogic.getStudentsForCourse(courseId), + usersLogic.getInstructorsForCourse(courseId)); + + // load question(s) + List allQuestions = getQuestionsForSession(feedbackSession, courseId, questionId); + RequestTracer.checkRemainingTime(); + + // load response(s) + Student student = isInstructor ? null : usersLogic.getStudentForEmail(courseId, userEmail); + Instructor instructor = isInstructor ? usersLogic.getInstructorForEmail(courseId, userEmail) : null; + List allResponses = new ArrayList<>(); + for (FeedbackQuestion question : allQuestions) { + // load viewable responses for students/instructors proactively + // this is cost-effective as in most of time responses for the whole session will not be viewable to individuals + List viewableResponses = isInstructor + ? getFeedbackResponsesToOrFromInstructorForQuestion(question, instructor) + : getViewableFeedbackResponsesForStudentForQuestion(question, student, roster); + allResponses.addAll(viewableResponses); + } + RequestTracer.checkRemainingTime(); + + return buildResultsBundle(false, feedbackSession, courseId, null, questionId, isInstructor, userEmail, + instructor, student, roster, allQuestions, allResponses); + } + + /** + * Builds viewable missing responses for the session for instructor. + * + * @param instructor the instructor + * @param responseGiverVisibilityTable + * the giver visibility table which will be updated with the visibility of missing responses + * @param responseRecipientVisibilityTable + * the recipient visibility table which will be updated with the visibility of missing responses + * @param relatedQuestions the relevant questions + * @param existingResponses existing responses + * @param courseRoster the course roster + * @param section if not null, will only build missing responses for the section + * @return a list of missing responses for the session. + */ + private List buildMissingResponses( + String courseId, FeedbackSession feedbackSession, Instructor instructor, + Map responseGiverVisibilityTable, Map responseRecipientVisibilityTable, + List relatedQuestions, + List existingResponses, SqlCourseRoster courseRoster, @Nullable String section) { + + // first get all possible giver recipient pairs + Map>> questionCompleteGiverRecipientMap = new HashMap<>(); + for (FeedbackQuestion feedbackQuestion : relatedQuestions) { + if (feedbackQuestion.getQuestionDetailsCopy().shouldGenerateMissingResponses(feedbackQuestion)) { + questionCompleteGiverRecipientMap.put(feedbackQuestion, + fqLogic.buildCompleteGiverRecipientMap(feedbackQuestion, courseRoster)); + } else { + questionCompleteGiverRecipientMap.put(feedbackQuestion, new HashMap<>()); + } + } + + // remove the existing responses in those pairs + for (FeedbackResponse existingResponse : existingResponses) { + Map> currGiverRecipientMap = + questionCompleteGiverRecipientMap.get(existingResponse.getFeedbackQuestion()); + if (!currGiverRecipientMap.containsKey(existingResponse.getGiver())) { + continue; + } + currGiverRecipientMap.get(existingResponse.getGiver()).remove(existingResponse.getRecipient()); + } + + List missingResponses = new ArrayList<>(); + // build dummy responses + for (Map.Entry>> currGiverRecipientMapEntry + : questionCompleteGiverRecipientMap.entrySet()) { + FeedbackQuestion correspondingQuestion = currGiverRecipientMapEntry.getKey(); + + for (Map.Entry> giverRecipientEntry + : currGiverRecipientMapEntry.getValue().entrySet()) { + // giver + String giverIdentifier = giverRecipientEntry.getKey(); + SqlCourseRoster.ParticipantInfo giverInfo = courseRoster.getInfoForIdentifier(giverIdentifier); + + for (String recipientIdentifier : giverRecipientEntry.getValue()) { + // recipient + SqlCourseRoster.ParticipantInfo recipientInfo = courseRoster.getInfoForIdentifier(recipientIdentifier); + + // skip responses not in current section + if (section != null + && !giverInfo.getSectionName().equals(section) + && !recipientInfo.getSectionName().equals(section)) { + continue; + } + + FeedbackResponse missingResponse = FeedbackResponse.makeResponse( + correspondingQuestion, + giverIdentifier, + giverInfo.getSection(), + recipientIdentifier, + recipientInfo.getSection(), + new FeedbackTextResponseDetails("No Response") + ); + + // check visibility of the missing response + boolean isVisibleResponse = isResponseVisibleForUser( + instructor.getEmail(), true, null, Collections.emptySet(), + missingResponse, correspondingQuestion, instructor); + if (!isVisibleResponse) { + continue; + } + + // generate giver/recipient name visibility table + responseGiverVisibilityTable.put(missingResponse, + isNameVisibleToUser(correspondingQuestion, missingResponse, + instructor.getEmail(), true, true, courseRoster)); + responseRecipientVisibilityTable.put(missingResponse, + isNameVisibleToUser(correspondingQuestion, missingResponse, + instructor.getEmail(), true, false, courseRoster)); + missingResponses.add(missingResponse); + } + } + } + + return missingResponses; + } + + + /** + * Checks whether the giver name of a response is visible to an user. + */ + public boolean isNameVisibleToUser( + FeedbackQuestion question, + FeedbackResponse response, + String userEmail, + boolean isInstructor, boolean isGiverName, SqlCourseRoster roster) { + + if (question == null) { + return false; + } + + // Early return if user is giver + if (question.getGiverType() == FeedbackParticipantType.TEAMS) { + // if response is given by team, then anyone in the team can see the response + if (roster.isStudentInTeam(userEmail, response.getGiver())) { + return true; + } + } else { + if (response.getGiver().equals(userEmail)) { + return true; + } + } + + return isFeedbackParticipantNameVisibleToUser(question, response, + userEmail, isInstructor, isGiverName, roster); + } + + private boolean isFeedbackParticipantNameVisibleToUser( + FeedbackQuestion question, FeedbackResponse response, + String userEmail, boolean isInstructor, boolean isGiverName, SqlCourseRoster roster) { + List showNameTo = isGiverName + ? question.getShowGiverNameTo() + : question.getShowRecipientNameTo(); + for (FeedbackParticipantType type : showNameTo) { + switch (type) { + case INSTRUCTORS: + if (roster.getInstructorForEmail(userEmail) != null && isInstructor) { + return true; + } + break; + case OWN_TEAM_MEMBERS: + case OWN_TEAM_MEMBERS_INCLUDING_SELF: + // Refers to Giver's Team Members + if (roster.isStudentsInSameTeam(response.getGiver(), userEmail)) { + return true; + } + break; + case RECEIVER: + // Response to team + if (question.getRecipientType().isTeam()) { + if (roster.isStudentInTeam(userEmail, response.getRecipient())) { + // this is a team name + return true; + } + break; + // Response to individual + } else if (response.getRecipient().equals(userEmail)) { + return true; + } else { + break; + } + case RECEIVER_TEAM_MEMBERS: + // Response to team; recipient = teamName + if (question.getRecipientType().isTeam()) { + if (roster.isStudentInTeam(userEmail, response.getRecipient())) { + // this is a team name + return true; + } + break; + } else if (roster.isStudentsInSameTeam(response.getRecipient(), userEmail)) { + // Response to individual + return true; + } + break; + case STUDENTS: + if (roster.isStudentInCourse(userEmail)) { + return true; + } + break; + default: + assert false : "Invalid FeedbackParticipantType for showNameTo in " + + "FeedbackResponseLogic.isFeedbackParticipantNameVisibleToUser()"; + break; + } + } + return false; + } + + private boolean isResponseVisibleForUser( + String userEmail, boolean isInstructor, Student student, + Set studentsEmailInTeam, FeedbackResponse response, + FeedbackQuestion relatedQuestion, Instructor instructor) { + + boolean isVisibleResponse = false; + if (isInstructor && relatedQuestion.isResponseVisibleTo(FeedbackParticipantType.INSTRUCTORS) + || response.getRecipient().equals(userEmail) + && relatedQuestion.isResponseVisibleTo(FeedbackParticipantType.RECEIVER) + || response.getGiver().equals(userEmail) + || !isInstructor && relatedQuestion.isResponseVisibleTo(FeedbackParticipantType.STUDENTS)) { + isVisibleResponse = true; + } else if (studentsEmailInTeam != null && !isInstructor) { + if ((relatedQuestion.getRecipientType() == FeedbackParticipantType.TEAMS + || relatedQuestion.getRecipientType() == FeedbackParticipantType.TEAMS_IN_SAME_SECTION + || relatedQuestion.getRecipientType() == FeedbackParticipantType.TEAMS_EXCLUDING_SELF) + && relatedQuestion.isResponseVisibleTo(FeedbackParticipantType.RECEIVER) + && response.getRecipient().equals(student.getTeamName())) { + isVisibleResponse = true; + } else if (relatedQuestion.getGiverType() == FeedbackParticipantType.TEAMS + && response.getGiver().equals(student.getTeamName())) { + isVisibleResponse = true; + } else if (relatedQuestion.isResponseVisibleTo(FeedbackParticipantType.OWN_TEAM_MEMBERS) + && studentsEmailInTeam.contains(response.getGiver())) { + isVisibleResponse = true; + } else if (relatedQuestion.isResponseVisibleTo(FeedbackParticipantType.RECEIVER_TEAM_MEMBERS) + && studentsEmailInTeam.contains(response.getRecipient())) { + isVisibleResponse = true; + } + } + if (isVisibleResponse && instructor != null) { + boolean isGiverSectionRestricted = + !instructor.isAllowedForPrivilege(response.getGiverSection().getName(), + response.getFeedbackSessionName(), + Const.InstructorPermissions.CAN_VIEW_SESSION_IN_SECTIONS); + // If instructors are not restricted to view the giver's section, + // they are allowed to view responses to GENERAL, subject to visibility options + boolean isRecipientSectionRestricted = + relatedQuestion.getRecipientType() != FeedbackParticipantType.NONE + && !instructor.isAllowedForPrivilege(response.getRecipientSection().getName(), + response.getFeedbackSessionName(), + Const.InstructorPermissions.CAN_VIEW_SESSION_IN_SECTIONS); + + boolean isNotAllowedForInstructor = isGiverSectionRestricted || isRecipientSectionRestricted; + if (isNotAllowedForInstructor) { + isVisibleResponse = false; + } + } + return isVisibleResponse; + } + + + /** + * Gets all responses for a session. + */ + List getFeedbackResponsesForSession( + FeedbackSession feedbackSession, String courseId) { + return frDb.getFeedbackResponsesForSession(feedbackSession, courseId); + } + + /** + * Gets all responses given to/from a section in a feedback session in a course. + * + * @param feedbackSession the session + * @param courseId the course ID of the session + * @param section if null, will retrieve all responses in the session + * @param fetchType if not null, will retrieve responses by giver, receiver sections, or both + * @return a list of responses + */ + public List getFeedbackResponsesForSessionInSection( + FeedbackSession feedbackSession, String courseId, @Nullable String section, + @Nullable FeedbackResultFetchType fetchType) { + if (section == null) { + return getFeedbackResponsesForSession(feedbackSession, courseId); + } + return frDb.getFeedbackResponsesForSessionInSection(feedbackSession, courseId, section, fetchType); + } + + /** + * Gets all responses for a question. + */ + public List getFeedbackResponsesForQuestion(UUID feedbackQuestionId) { + return frDb.getFeedbackResponsesForQuestion(feedbackQuestionId); + } + + + /** + * Gets all responses given to/from a section for a question. + * + * @param feedbackQuestionId the question UUID + * @param section if null, will retrieve all responses for the question + * @return a list of responses + */ + public List getFeedbackResponsesForQuestionInSection( + UUID feedbackQuestionId, @Nullable String section, FeedbackResultFetchType fetchType) { + if (section == null) { + return getFeedbackResponsesForQuestion(feedbackQuestionId); + } + return frDb.getFeedbackResponsesForQuestionInSection(feedbackQuestionId, section, fetchType); + } + + /** + * Returns feedback responses given/received by an instructor. + */ + private List getFeedbackResponsesToOrFromInstructorForQuestion( + FeedbackQuestion question, Instructor instructor) { + HashSet viewableResponses = new HashSet<>(); + + // Add responses that the instructor submitted him/herself + if (question.getGiverType() == FeedbackParticipantType.INSTRUCTORS) { + viewableResponses.addAll( + getFeedbackResponsesFromGiverForQuestion(question.getId(), instructor.getEmail()) + ); + } + + // Add responses that user is a receiver of when response is visible to receiver or instructors + if (question.getRecipientType() == FeedbackParticipantType.INSTRUCTORS + && (question.isResponseVisibleTo(FeedbackParticipantType.RECEIVER) + || question.isResponseVisibleTo(FeedbackParticipantType.INSTRUCTORS))) { + viewableResponses.addAll( + getFeedbackResponsesForRecipientForQuestion(question.getId(), instructor.getEmail()) + ); + } + + return new ArrayList<>(viewableResponses); + } + + /** + * Returns viewable feedback responses for a student. + */ + private List getViewableFeedbackResponsesForStudentForQuestion( + FeedbackQuestion question, Student student, SqlCourseRoster courseRoster) { + HashSet viewableResponses = new HashSet<>(); + + // Add responses that the student submitted him/herself + if (question.getGiverType() != FeedbackParticipantType.INSTRUCTORS) { + viewableResponses.addAll( + getFeedbackResponsesFromGiverForQuestion(question.getId(), student.getEmail()) + ); + } + + // Add responses that user is a receiver of when response is visible to receiver + if (question.getRecipientType() != FeedbackParticipantType.INSTRUCTORS + && question.isResponseVisibleTo(FeedbackParticipantType.RECEIVER)) { + viewableResponses.addAll( + getFeedbackResponsesForRecipientForQuestion(question.getId(), student.getEmail()) + ); + } + + if (question.isResponseVisibleTo(FeedbackParticipantType.STUDENTS)) { + viewableResponses.addAll(getFeedbackResponsesForQuestion(question.getId())); + + // Early return as STUDENTS covers all cases below. + return new ArrayList<>(viewableResponses); + } + + if (question.getRecipientType().isTeam() + && question.isResponseVisibleTo(FeedbackParticipantType.RECEIVER)) { + viewableResponses.addAll( + getFeedbackResponsesForRecipientForQuestion(question.getId(), student.getTeamName()) + ); + } + + if (question.getGiverType() == FeedbackParticipantType.TEAMS + || question.isResponseVisibleTo(FeedbackParticipantType.OWN_TEAM_MEMBERS)) { + viewableResponses.addAll( + getFeedbackResponsesFromTeamForQuestion( + question.getId(), question.getCourseId(), student.getTeamName(), courseRoster)); + } + + if (question.isResponseVisibleTo(FeedbackParticipantType.RECEIVER_TEAM_MEMBERS)) { + for (Student studentInTeam : courseRoster.getTeamToMembersTable().get(student.getTeamName())) { + if (studentInTeam.getEmail().equals(student.getEmail())) { + continue; + } + List responses = + getFeedbackResponsesForRecipientForQuestion(question.getId(), studentInTeam.getEmail()); + viewableResponses.addAll(responses); + } + } + + return new ArrayList<>(viewableResponses); + } + + /** + * Gets all responses received by a user for a question. + */ + private List getFeedbackResponsesForRecipientForQuestion( + UUID feedbackQuestionId, String userEmail) { + return frDb.getFeedbackResponsesForRecipientForQuestion(feedbackQuestionId, userEmail); + } } diff --git a/src/main/java/teammates/storage/sqlentity/FeedbackResponse.java b/src/main/java/teammates/storage/sqlentity/FeedbackResponse.java index 983e12126ba..555552995d8 100644 --- a/src/main/java/teammates/storage/sqlentity/FeedbackResponse.java +++ b/src/main/java/teammates/storage/sqlentity/FeedbackResponse.java @@ -209,6 +209,14 @@ public void setUpdatedAt(Instant updatedAt) { this.updatedAt = updatedAt; } + public FeedbackSession getFeedbackSession() { + return feedbackQuestion.getFeedbackSession(); + } + + public String getFeedbackSessionName() { + return getFeedbackSession().getName(); + } + @Override public List getInvalidityInfo() { return new ArrayList<>(); diff --git a/src/main/java/teammates/storage/sqlentity/FeedbackResponseComment.java b/src/main/java/teammates/storage/sqlentity/FeedbackResponseComment.java index be969ae6eb2..76c850f3fe2 100644 --- a/src/main/java/teammates/storage/sqlentity/FeedbackResponseComment.java +++ b/src/main/java/teammates/storage/sqlentity/FeedbackResponseComment.java @@ -206,6 +206,13 @@ public void setLastEditorEmail(String lastEditorEmail) { this.lastEditorEmail = lastEditorEmail; } + /** + * Returns true if the response comment is visible to the given participant type. + */ + public boolean isVisibleTo(FeedbackParticipantType viewerType) { + return showCommentTo.contains(viewerType); + } + @Override public List getInvalidityInfo() { List errors = new ArrayList<>(); diff --git a/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java b/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java index 17d409c8175..6a56669afab 100644 --- a/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java +++ b/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java @@ -1,7 +1,10 @@ package teammates.ui.webapi; +import java.util.UUID; + import teammates.common.datatransfer.FeedbackResultFetchType; import teammates.common.datatransfer.SessionResultsBundle; +import teammates.common.datatransfer.SqlSessionResultsBundle; import teammates.common.datatransfer.attributes.FeedbackSessionAttributes; import teammates.common.datatransfer.attributes.InstructorAttributes; import teammates.common.datatransfer.attributes.StudentAttributes; @@ -100,44 +103,86 @@ public JsonResult execute() { FeedbackResultFetchType fetchType = FeedbackResultFetchType.parseFetchType( getRequestParamValue(Const.ParamsNames.FEEDBACK_RESULTS_SECTION_BY_GIVER_RECEIVER)); - SessionResultsBundle bundle; - InstructorAttributes instructor; - StudentAttributes student; Intent intent = Intent.valueOf(getNonNullRequestParamValue(Const.ParamsNames.INTENT)); - switch (intent) { - case FULL_DETAIL: - instructor = logic.getInstructorForGoogleId(courseId, userInfo.id); - - bundle = logic.getSessionResultsForCourse(feedbackSessionName, courseId, instructor.getEmail(), - questionId, selectedSection, fetchType); - return new JsonResult(SessionResultsData.initForInstructor(bundle)); - case INSTRUCTOR_RESULT: - // Section name filter is not applicable here - instructor = getPossiblyUnregisteredInstructor(courseId); - - bundle = logic.getSessionResultsForUser(feedbackSessionName, courseId, instructor.getEmail(), - true, questionId); - - // Build a fake student object, as the results will be displayed as if they are displayed to a student - student = StudentAttributes.builder(instructor.getCourseId(), instructor.getEmail()) - .withTeamName(Const.USER_TEAM_FOR_INSTRUCTOR) - .build(); - - return new JsonResult(SessionResultsData.initForStudent(bundle, student)); - case STUDENT_RESULT: - // Section name filter is not applicable here - student = getPossiblyUnregisteredStudent(courseId); - - bundle = logic.getSessionResultsForUser(feedbackSessionName, courseId, student.getEmail(), - false, questionId); - - return new JsonResult(SessionResultsData.initForStudent(bundle, student)); - case INSTRUCTOR_SUBMISSION: - case STUDENT_SUBMISSION: - throw new InvalidHttpParameterException("Invalid intent for this action"); - default: - throw new InvalidHttpParameterException("Unknown intent " + intent); + + + if (isCourseMigrated(courseId)) { + Instructor instructor; + Student student; + FeedbackSession feedbackSession = getNonNullSqlFeedbackSession(feedbackSessionName, courseId); + SqlSessionResultsBundle bundle; + UUID questionUuid = UUID.fromString(questionId); + switch (intent) { + case FULL_DETAIL: + instructor = sqlLogic.getInstructorByGoogleId(courseId, userInfo.id); + + bundle = sqlLogic.getSessionResultsForCourse(feedbackSession, courseId, instructor.getEmail(), + questionUuid, selectedSection, fetchType); + return new JsonResult(SessionResultsData.initForInstructor(bundle)); + case INSTRUCTOR_RESULT: + // Section name filter is not applicable here + instructor = getPossiblyUnregisteredSqlInstructor(courseId); + + bundle = sqlLogic.getSessionResultsForUser(feedbackSession, courseId, instructor.getEmail(), + true, questionUuid); + + // Build a fake student object, as the results will be displayed as if they are displayed to a student + student = new Student(instructor.getCourse(), instructor.getName(), instructor.getEmail(), ""); + student.setTeam(instructor.getTeam()); + + return new JsonResult(SessionResultsData.initForStudent(bundle, student)); + case STUDENT_RESULT: + // Section name filter is not applicable here + student = getPossiblyUnregisteredSqlStudent(courseId); + + bundle = sqlLogic.getSessionResultsForUser(feedbackSession, courseId, student.getEmail(), + false, questionUuid); + + return new JsonResult(SessionResultsData.initForStudent(bundle, student)); + case INSTRUCTOR_SUBMISSION: + case STUDENT_SUBMISSION: + throw new InvalidHttpParameterException("Invalid intent for this action"); + default: + throw new InvalidHttpParameterException("Unknown intent " + intent); + } + } else { + InstructorAttributes instructor; + StudentAttributes student; + SessionResultsBundle bundle; + switch (intent) { + case FULL_DETAIL: + instructor = logic.getInstructorForGoogleId(courseId, userInfo.id); + + bundle = logic.getSessionResultsForCourse(feedbackSessionName, courseId, instructor.getEmail(), + questionId, selectedSection, fetchType); + return new JsonResult(SessionResultsData.initForInstructor(bundle)); + case INSTRUCTOR_RESULT: + // Section name filter is not applicable here + instructor = getPossiblyUnregisteredInstructor(courseId); + + bundle = logic.getSessionResultsForUser(feedbackSessionName, courseId, instructor.getEmail(), + true, questionId); + + // Build a fake student object, as the results will be displayed as if they are displayed to a student + student = StudentAttributes.builder(instructor.getCourseId(), instructor.getEmail()) + .withTeamName(Const.USER_TEAM_FOR_INSTRUCTOR) + .build(); + + return new JsonResult(SessionResultsData.initForStudent(bundle, student)); + case STUDENT_RESULT: + // Section name filter is not applicable here + student = getPossiblyUnregisteredStudent(courseId); + + bundle = logic.getSessionResultsForUser(feedbackSessionName, courseId, student.getEmail(), + false, questionId); + + return new JsonResult(SessionResultsData.initForStudent(bundle, student)); + case INSTRUCTOR_SUBMISSION: + case STUDENT_SUBMISSION: + throw new InvalidHttpParameterException("Invalid intent for this action"); + default: + throw new InvalidHttpParameterException("Unknown intent " + intent); + } } } - } From 97ae4b5fa7026d49e611fe84b852e62587eccdae Mon Sep 17 00:00:00 2001 From: Xenos F Date: Thu, 15 Feb 2024 09:31:11 +0800 Subject: [PATCH 10/42] Refactor Datastore and SQL action logic out to separate methods --- .../ui/webapi/GetSessionResultsAction.java | 278 +++++++++--------- 1 file changed, 147 insertions(+), 131 deletions(-) diff --git a/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java b/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java index 6a56669afab..2f605ba7e67 100644 --- a/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java +++ b/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java @@ -32,63 +32,71 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException { Intent intent = Intent.valueOf(getNonNullRequestParamValue(Const.ParamsNames.INTENT)); if (isCourseMigrated(courseId)) { - FeedbackSession feedbackSession = getNonNullSqlFeedbackSession(feedbackSessionName, courseId); - - switch (intent) { - case FULL_DETAIL: - gateKeeper.verifyLoggedInUserPrivileges(userInfo); - Instructor instructor = sqlLogic.getInstructorByGoogleId(courseId, userInfo.getId()); - gateKeeper.verifyAccessible(instructor, feedbackSession); - break; - case INSTRUCTOR_RESULT: - instructor = getPossiblyUnregisteredSqlInstructor(courseId); - gateKeeper.verifyAccessible(instructor, feedbackSession); - if (!feedbackSession.isPublished()) { - throw new UnauthorizedAccessException("This feedback session is not yet published.", true); - } - break; - case STUDENT_RESULT: - Student student = getPossiblyUnregisteredSqlStudent(courseId); - gateKeeper.verifyAccessible(student, feedbackSession); - if (!feedbackSession.isPublished()) { - throw new UnauthorizedAccessException("This feedback session is not yet published.", true); - } - break; - case INSTRUCTOR_SUBMISSION: - case STUDENT_SUBMISSION: - throw new InvalidHttpParameterException("Invalid intent for this action"); - default: - throw new InvalidHttpParameterException("Unknown intent " + intent); - } + checkSpecificAccessControlSql(courseId, feedbackSessionName, intent); } else { - FeedbackSessionAttributes feedbackSession = getNonNullFeedbackSession(feedbackSessionName, courseId); - - switch (intent) { - case FULL_DETAIL: - gateKeeper.verifyLoggedInUserPrivileges(userInfo); - InstructorAttributes instructor = logic.getInstructorForGoogleId(courseId, userInfo.getId()); - gateKeeper.verifyAccessible(instructor, feedbackSession); - break; - case INSTRUCTOR_RESULT: - instructor = getPossiblyUnregisteredInstructor(courseId); - gateKeeper.verifyAccessible(instructor, feedbackSession); - if (!feedbackSession.isPublished()) { - throw new UnauthorizedAccessException("This feedback session is not yet published.", true); - } - break; - case STUDENT_RESULT: - StudentAttributes student = getPossiblyUnregisteredStudent(courseId); - gateKeeper.verifyAccessible(student, feedbackSession); - if (!feedbackSession.isPublished()) { - throw new UnauthorizedAccessException("This feedback session is not yet published.", true); - } - break; - case INSTRUCTOR_SUBMISSION: - case STUDENT_SUBMISSION: - throw new InvalidHttpParameterException("Invalid intent for this action"); - default: - throw new InvalidHttpParameterException("Unknown intent " + intent); + checkSpecificAccessControlDatastore(courseId, feedbackSessionName, intent); + } + } + + private void checkSpecificAccessControlDatastore(String courseId, String feedbackSessionName, Intent intent) throws UnauthorizedAccessException { + FeedbackSessionAttributes feedbackSession = getNonNullFeedbackSession(feedbackSessionName, courseId); + + switch (intent) { + case FULL_DETAIL: + gateKeeper.verifyLoggedInUserPrivileges(userInfo); + InstructorAttributes instructor = logic.getInstructorForGoogleId(courseId, userInfo.getId()); + gateKeeper.verifyAccessible(instructor, feedbackSession); + break; + case INSTRUCTOR_RESULT: + instructor = getPossiblyUnregisteredInstructor(courseId); + gateKeeper.verifyAccessible(instructor, feedbackSession); + if (!feedbackSession.isPublished()) { + throw new UnauthorizedAccessException("This feedback session is not yet published.", true); + } + break; + case STUDENT_RESULT: + StudentAttributes student = getPossiblyUnregisteredStudent(courseId); + gateKeeper.verifyAccessible(student, feedbackSession); + if (!feedbackSession.isPublished()) { + throw new UnauthorizedAccessException("This feedback session is not yet published.", true); + } + break; + case INSTRUCTOR_SUBMISSION: + case STUDENT_SUBMISSION: + throw new InvalidHttpParameterException("Invalid intent for this action"); + default: + throw new InvalidHttpParameterException("Unknown intent " + intent); + } + } + + private void checkSpecificAccessControlSql(String courseId, String feedbackSessionName, Intent intent) throws UnauthorizedAccessException { + FeedbackSession feedbackSession = getNonNullSqlFeedbackSession(feedbackSessionName, courseId); + + switch (intent) { + case FULL_DETAIL: + gateKeeper.verifyLoggedInUserPrivileges(userInfo); + Instructor instructor = sqlLogic.getInstructorByGoogleId(courseId, userInfo.getId()); + gateKeeper.verifyAccessible(instructor, feedbackSession); + break; + case INSTRUCTOR_RESULT: + instructor = getPossiblyUnregisteredSqlInstructor(courseId); + gateKeeper.verifyAccessible(instructor, feedbackSession); + if (!feedbackSession.isPublished()) { + throw new UnauthorizedAccessException("This feedback session is not yet published.", true); + } + break; + case STUDENT_RESULT: + Student student = getPossiblyUnregisteredSqlStudent(courseId); + gateKeeper.verifyAccessible(student, feedbackSession); + if (!feedbackSession.isPublished()) { + throw new UnauthorizedAccessException("This feedback session is not yet published.", true); } + break; + case INSTRUCTOR_SUBMISSION: + case STUDENT_SUBMISSION: + throw new InvalidHttpParameterException("Invalid intent for this action"); + default: + throw new InvalidHttpParameterException("Unknown intent " + intent); } } @@ -104,85 +112,93 @@ public JsonResult execute() { getRequestParamValue(Const.ParamsNames.FEEDBACK_RESULTS_SECTION_BY_GIVER_RECEIVER)); Intent intent = Intent.valueOf(getNonNullRequestParamValue(Const.ParamsNames.INTENT)); - if (isCourseMigrated(courseId)) { - Instructor instructor; - Student student; - FeedbackSession feedbackSession = getNonNullSqlFeedbackSession(feedbackSessionName, courseId); - SqlSessionResultsBundle bundle; - UUID questionUuid = UUID.fromString(questionId); - switch (intent) { - case FULL_DETAIL: - instructor = sqlLogic.getInstructorByGoogleId(courseId, userInfo.id); - - bundle = sqlLogic.getSessionResultsForCourse(feedbackSession, courseId, instructor.getEmail(), - questionUuid, selectedSection, fetchType); - return new JsonResult(SessionResultsData.initForInstructor(bundle)); - case INSTRUCTOR_RESULT: - // Section name filter is not applicable here - instructor = getPossiblyUnregisteredSqlInstructor(courseId); - - bundle = sqlLogic.getSessionResultsForUser(feedbackSession, courseId, instructor.getEmail(), - true, questionUuid); - - // Build a fake student object, as the results will be displayed as if they are displayed to a student - student = new Student(instructor.getCourse(), instructor.getName(), instructor.getEmail(), ""); - student.setTeam(instructor.getTeam()); - - return new JsonResult(SessionResultsData.initForStudent(bundle, student)); - case STUDENT_RESULT: - // Section name filter is not applicable here - student = getPossiblyUnregisteredSqlStudent(courseId); - - bundle = sqlLogic.getSessionResultsForUser(feedbackSession, courseId, student.getEmail(), - false, questionUuid); - - return new JsonResult(SessionResultsData.initForStudent(bundle, student)); - case INSTRUCTOR_SUBMISSION: - case STUDENT_SUBMISSION: - throw new InvalidHttpParameterException("Invalid intent for this action"); - default: - throw new InvalidHttpParameterException("Unknown intent " + intent); - } + return executeWithSql(courseId, feedbackSessionName, questionId, selectedSection, fetchType, intent); } else { - InstructorAttributes instructor; - StudentAttributes student; - SessionResultsBundle bundle; - switch (intent) { - case FULL_DETAIL: - instructor = logic.getInstructorForGoogleId(courseId, userInfo.id); - - bundle = logic.getSessionResultsForCourse(feedbackSessionName, courseId, instructor.getEmail(), - questionId, selectedSection, fetchType); - return new JsonResult(SessionResultsData.initForInstructor(bundle)); - case INSTRUCTOR_RESULT: - // Section name filter is not applicable here - instructor = getPossiblyUnregisteredInstructor(courseId); - - bundle = logic.getSessionResultsForUser(feedbackSessionName, courseId, instructor.getEmail(), - true, questionId); - - // Build a fake student object, as the results will be displayed as if they are displayed to a student - student = StudentAttributes.builder(instructor.getCourseId(), instructor.getEmail()) - .withTeamName(Const.USER_TEAM_FOR_INSTRUCTOR) - .build(); - - return new JsonResult(SessionResultsData.initForStudent(bundle, student)); - case STUDENT_RESULT: - // Section name filter is not applicable here - student = getPossiblyUnregisteredStudent(courseId); - - bundle = logic.getSessionResultsForUser(feedbackSessionName, courseId, student.getEmail(), - false, questionId); - - return new JsonResult(SessionResultsData.initForStudent(bundle, student)); - case INSTRUCTOR_SUBMISSION: - case STUDENT_SUBMISSION: - throw new InvalidHttpParameterException("Invalid intent for this action"); - default: - throw new InvalidHttpParameterException("Unknown intent " + intent); - } + return executeWithDatastore(courseId, feedbackSessionName, questionId, selectedSection, fetchType, intent); + } + } + + private JsonResult executeWithDatastore(String courseId, String feedbackSessionName, String questionId, String selectedSection, FeedbackResultFetchType fetchType, Intent intent) { + InstructorAttributes instructor; + StudentAttributes student; + SessionResultsBundle bundle; + switch (intent) { + case FULL_DETAIL: + instructor = logic.getInstructorForGoogleId(courseId, userInfo.id); + + bundle = logic.getSessionResultsForCourse(feedbackSessionName, courseId, instructor.getEmail(), + questionId, selectedSection, fetchType); + return new JsonResult(SessionResultsData.initForInstructor(bundle)); + case INSTRUCTOR_RESULT: + // Section name filter is not applicable here + instructor = getPossiblyUnregisteredInstructor(courseId); + + bundle = logic.getSessionResultsForUser(feedbackSessionName, courseId, instructor.getEmail(), + true, questionId); + + // Build a fake student object, as the results will be displayed as if they are displayed to a student + student = StudentAttributes.builder(instructor.getCourseId(), instructor.getEmail()) + .withTeamName(Const.USER_TEAM_FOR_INSTRUCTOR) + .build(); + + return new JsonResult(SessionResultsData.initForStudent(bundle, student)); + case STUDENT_RESULT: + // Section name filter is not applicable here + student = getPossiblyUnregisteredStudent(courseId); + + bundle = logic.getSessionResultsForUser(feedbackSessionName, courseId, student.getEmail(), + false, questionId); + + return new JsonResult(SessionResultsData.initForStudent(bundle, student)); + case INSTRUCTOR_SUBMISSION: + case STUDENT_SUBMISSION: + throw new InvalidHttpParameterException("Invalid intent for this action"); + default: + throw new InvalidHttpParameterException("Unknown intent " + intent); } } + + private JsonResult executeWithSql(String courseId, String feedbackSessionName, String questionId, String selectedSection, FeedbackResultFetchType fetchType, Intent intent) { + Instructor instructor; + Student student; + FeedbackSession feedbackSession = getNonNullSqlFeedbackSession(feedbackSessionName, courseId); + SqlSessionResultsBundle bundle; + UUID questionUuid = UUID.fromString(questionId); + switch (intent) { + case FULL_DETAIL: + instructor = sqlLogic.getInstructorByGoogleId(courseId, userInfo.id); + + bundle = sqlLogic.getSessionResultsForCourse(feedbackSession, courseId, instructor.getEmail(), + questionUuid, selectedSection, fetchType); + return new JsonResult(SessionResultsData.initForInstructor(bundle)); + case INSTRUCTOR_RESULT: + // Section name filter is not applicable here + instructor = getPossiblyUnregisteredSqlInstructor(courseId); + + bundle = sqlLogic.getSessionResultsForUser(feedbackSession, courseId, instructor.getEmail(), + true, questionUuid); + + // Build a fake student object, as the results will be displayed as if they are displayed to a student + student = new Student(instructor.getCourse(), instructor.getName(), instructor.getEmail(), ""); + student.setTeam(instructor.getTeam()); + + return new JsonResult(SessionResultsData.initForStudent(bundle, student)); + case STUDENT_RESULT: + // Section name filter is not applicable here + student = getPossiblyUnregisteredSqlStudent(courseId); + + bundle = sqlLogic.getSessionResultsForUser(feedbackSession, courseId, student.getEmail(), + false, questionUuid); + + return new JsonResult(SessionResultsData.initForStudent(bundle, student)); + case INSTRUCTOR_SUBMISSION: + case STUDENT_SUBMISSION: + throw new InvalidHttpParameterException("Invalid intent for this action"); + default: + throw new InvalidHttpParameterException("Unknown intent " + intent); + } + } + } From 51f738f7b14e50773e06a40ec3a85a7589dd3fbe Mon Sep 17 00:00:00 2001 From: Xenos F Date: Thu, 15 Feb 2024 09:44:14 +0800 Subject: [PATCH 11/42] Fix checkstyle errors --- .../java/teammates/common/util/Const.java | 1 - .../java/teammates/sqllogic/api/Logic.java | 2 -- .../sqllogic/core/FeedbackQuestionsLogic.java | 18 +++++----- .../core/FeedbackResponseCommentsLogic.java | 2 -- .../sqllogic/core/FeedbackResponsesLogic.java | 34 +++++++++---------- .../storage/sqlentity/Instructor.java | 5 +-- .../ui/output/SessionResultsData.java | 8 ----- .../ui/webapi/GetSessionResultsAction.java | 20 +++++++---- 8 files changed, 42 insertions(+), 48 deletions(-) diff --git a/src/main/java/teammates/common/util/Const.java b/src/main/java/teammates/common/util/Const.java index bcfe84f6d5c..78e0ba0df35 100644 --- a/src/main/java/teammates/common/util/Const.java +++ b/src/main/java/teammates/common/util/Const.java @@ -6,7 +6,6 @@ import java.time.Instant; import teammates.storage.sqlentity.Section; -import teammates.storage.sqlentity.Team; /** * Stores constants that are widely used across classes. diff --git a/src/main/java/teammates/sqllogic/api/Logic.java b/src/main/java/teammates/sqllogic/api/Logic.java index f21c0078a41..70211dd9d0c 100644 --- a/src/main/java/teammates/sqllogic/api/Logic.java +++ b/src/main/java/teammates/sqllogic/api/Logic.java @@ -8,7 +8,6 @@ import javax.annotation.Nullable; -import teammates.common.datatransfer.DataBundle; import teammates.common.datatransfer.FeedbackQuestionRecipient; import teammates.common.datatransfer.FeedbackResultFetchType; import teammates.common.datatransfer.NotificationStyle; @@ -1095,7 +1094,6 @@ public List getFeedbackQuestionsForInstructors( return feedbackQuestionsLogic.getFeedbackQuestionsForInstructors(feedbackSession, instructorEmail); } - /** * Gets the session result for a feedback session. * diff --git a/src/main/java/teammates/sqllogic/core/FeedbackQuestionsLogic.java b/src/main/java/teammates/sqllogic/core/FeedbackQuestionsLogic.java index bf700b31b86..bb951853bc1 100644 --- a/src/main/java/teammates/sqllogic/core/FeedbackQuestionsLogic.java +++ b/src/main/java/teammates/sqllogic/core/FeedbackQuestionsLogic.java @@ -12,14 +12,9 @@ import javax.annotation.Nullable; -import teammates.common.datatransfer.CourseRoster; import teammates.common.datatransfer.FeedbackParticipantType; import teammates.common.datatransfer.FeedbackQuestionRecipient; import teammates.common.datatransfer.SqlCourseRoster; -import teammates.common.datatransfer.attributes.FeedbackQuestionAttributes; -import teammates.common.datatransfer.attributes.FeedbackSessionAttributes; -import teammates.common.datatransfer.attributes.InstructorAttributes; -import teammates.common.datatransfer.attributes.StudentAttributes; import teammates.common.datatransfer.questions.FeedbackMcqQuestionDetails; import teammates.common.datatransfer.questions.FeedbackMsqQuestionDetails; import teammates.common.datatransfer.questions.FeedbackQuestionDetails; @@ -652,7 +647,6 @@ public List getFeedbackQuestionForCourseWithType( .collect(Collectors.toList()); } - /** * Builds a complete giver to recipient map for a {@code relatedQuestion}. * @@ -688,7 +682,15 @@ public Map> buildCompleteGiverRecipientMap( // only happens when a session creator quits their course if (instructorGiver == null) { - instructorGiver = new Instructor(relatedQuestion.getCourse(), USER_NAME_FOR_SELF, possibleGiverEmail, false, USER_NAME_FOR_SELF, null, null); + instructorGiver = new Instructor( + relatedQuestion.getCourse(), + USER_NAME_FOR_SELF, + possibleGiverEmail, + false, + USER_NAME_FOR_SELF, + null, + null + ); } completeGiverRecipientMap @@ -705,7 +707,6 @@ public Map> buildCompleteGiverRecipientMap( return completeGiverRecipientMap; } - /** * Gets possible giver identifiers for a feedback question. * @@ -747,5 +748,4 @@ private List getPossibleGivers( return possibleGivers; } - } diff --git a/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java b/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java index 89808f6631e..6701fb60b48 100644 --- a/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java +++ b/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java @@ -135,7 +135,6 @@ public void updateFeedbackResponseCommentsForResponse(FeedbackResponse response) } } - /** * Gets all feedback response comments for session in a section. * @@ -167,7 +166,6 @@ public List getFeedbackResponseCommentForQuestionInSect return frcDb.getFeedbackResponseCommentsForQuestionInSection(questionId, section); } - /** * Verifies whether the comment is visible to certain user. * @return true/false diff --git a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java index 52ab88b1a1f..ada714a5801 100644 --- a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java +++ b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java @@ -462,7 +462,7 @@ public void updateFeedbackResponsesForChangingSection(Course course, String newE frcLogic.updateFeedbackResponseCommentsForResponse(response); } } - + private List getQuestionsForSession( FeedbackSession feedbackSession, String courseId, @Nullable UUID questionId) { if (questionId == null) { @@ -481,7 +481,8 @@ private SqlSessionResultsBundle buildResultsBundle( // load comment(s) List allComments; if (questionId == null) { - allComments = frcLogic.getFeedbackResponseCommentForSessionInSection(courseId, feedbackSession.getName(), section); + allComments = frcLogic.getFeedbackResponseCommentForSessionInSection( + courseId, feedbackSession.getName(), section); } else { allComments = frcLogic.getFeedbackResponseCommentForQuestionInSection(questionId, section); } @@ -586,10 +587,10 @@ private SqlSessionResultsBundle buildResultsBundle( public SqlSessionResultsBundle getSessionResultsForCourse( FeedbackSession feedbackSession, String courseId, String instructorEmail, @Nullable UUID questionId, @Nullable String section, @Nullable FeedbackResultFetchType fetchType) { - + SqlCourseRoster roster = new SqlCourseRoster( - usersLogic.getStudentsForCourse(courseId), - usersLogic.getInstructorsForCourse(courseId)); + usersLogic.getStudentsForCourse(courseId), + usersLogic.getInstructorsForCourse(courseId)); // load question(s) List allQuestions = getQuestionsForSession(feedbackSession, courseId, questionId); @@ -667,7 +668,8 @@ public SqlSessionResultsBundle getSessionResultsForUser( */ private List buildMissingResponses( String courseId, FeedbackSession feedbackSession, Instructor instructor, - Map responseGiverVisibilityTable, Map responseRecipientVisibilityTable, + Map responseGiverVisibilityTable, + Map responseRecipientVisibilityTable, List relatedQuestions, List existingResponses, SqlCourseRoster courseRoster, @Nullable String section) { @@ -716,12 +718,12 @@ private List buildMissingResponses( } FeedbackResponse missingResponse = FeedbackResponse.makeResponse( - correspondingQuestion, - giverIdentifier, - giverInfo.getSection(), - recipientIdentifier, - recipientInfo.getSection(), - new FeedbackTextResponseDetails("No Response") + correspondingQuestion, + giverIdentifier, + giverInfo.getSection(), + recipientIdentifier, + recipientInfo.getSection(), + new FeedbackTextResponseDetails("No Response") ); // check visibility of the missing response @@ -747,7 +749,6 @@ private List buildMissingResponses( return missingResponses; } - /** * Checks whether the giver name of a response is visible to an user. */ @@ -923,7 +924,6 @@ public List getFeedbackResponsesForQuestion(UUID feedbackQuest return frDb.getFeedbackResponsesForQuestion(feedbackQuestionId); } - /** * Gets all responses given to/from a section for a question. * @@ -944,7 +944,7 @@ public List getFeedbackResponsesForQuestionInSection( */ private List getFeedbackResponsesToOrFromInstructorForQuestion( FeedbackQuestion question, Instructor instructor) { - HashSet viewableResponses = new HashSet<>(); + Set viewableResponses = new HashSet<>(); // Add responses that the instructor submitted him/herself if (question.getGiverType() == FeedbackParticipantType.INSTRUCTORS) { @@ -970,7 +970,7 @@ private List getFeedbackResponsesToOrFromInstructorForQuestion */ private List getViewableFeedbackResponsesForStudentForQuestion( FeedbackQuestion question, Student student, SqlCourseRoster courseRoster) { - HashSet viewableResponses = new HashSet<>(); + Set viewableResponses = new HashSet<>(); // Add responses that the student submitted him/herself if (question.getGiverType() != FeedbackParticipantType.INSTRUCTORS) { @@ -1021,7 +1021,7 @@ private List getViewableFeedbackResponsesForStudentForQuestion return new ArrayList<>(viewableResponses); } - + /** * Gets all responses received by a user for a question. */ diff --git a/src/main/java/teammates/storage/sqlentity/Instructor.java b/src/main/java/teammates/storage/sqlentity/Instructor.java index e9d7ea56b98..24821a29f61 100644 --- a/src/main/java/teammates/storage/sqlentity/Instructor.java +++ b/src/main/java/teammates/storage/sqlentity/Instructor.java @@ -25,6 +25,9 @@ @Entity @Table(name = "Instructors") public class Instructor extends User { + + private static Team userTeam = new Team(null, Const.USER_TEAM_FOR_INSTRUCTOR); + @Column(nullable = false) private boolean isDisplayedToStudents; @@ -39,8 +42,6 @@ public class Instructor extends User { @Convert(converter = InstructorPrivilegesConverter.class) private InstructorPrivileges privileges; - private static Team userTeam = new Team(null, Const.USER_TEAM_FOR_INSTRUCTOR); - protected Instructor() { // required by Hibernate } diff --git a/src/main/java/teammates/ui/output/SessionResultsData.java b/src/main/java/teammates/ui/output/SessionResultsData.java index 61aa7c90022..dfea2e0ef77 100644 --- a/src/main/java/teammates/ui/output/SessionResultsData.java +++ b/src/main/java/teammates/ui/output/SessionResultsData.java @@ -72,7 +72,6 @@ public static SessionResultsData initForInstructor(SessionResultsBundle bundle) return sessionResultsData; } - /** * Factory method to construct API output for instructor. */ @@ -156,7 +155,6 @@ public static SessionResultsData initForStudent(SessionResultsBundle bundle, Stu return sessionResultsData; } - /** * Factory method to construct API output for student. */ @@ -281,7 +279,6 @@ private static ResponseOutput buildSingleResponseForStudent( .build(); } - private static ResponseOutput buildSingleResponseForStudent( FeedbackResponse response, SqlSessionResultsBundle bundle, Student student) { FeedbackQuestion question = response.getFeedbackQuestion(); @@ -452,7 +449,6 @@ private static ResponseOutput buildSingleResponseForInstructor( .build(); } - private static ResponseOutput buildSingleResponseForInstructor( FeedbackResponse response, SqlSessionResultsBundle bundle, boolean isMissingResponse) { // process giver @@ -546,7 +542,6 @@ private static String getGiverNameOfResponse(FeedbackResponseAttributes response return name; } - /** * Gets giver name of a response from the bundle. * @@ -592,7 +587,6 @@ private static String getRecipientNameOfResponse(FeedbackResponseAttributes resp return name; } - /** * Gets recipient name of a response from the bundle. * @@ -657,7 +651,6 @@ private static Queue buildComments(List buildComments(List feedbackResponseComments, SqlSessionResultsBundle bundle) { LinkedList outputs = new LinkedList<>(); @@ -989,7 +982,6 @@ static Builder builder(FeedbackResponseComment frc) { return new Builder(frc); } - @Nullable public String getCommentGiverName() { return commentGiverName; diff --git a/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java b/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java index 2f605ba7e67..70f780caa27 100644 --- a/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java +++ b/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java @@ -30,7 +30,7 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException { String courseId = getNonNullRequestParamValue(Const.ParamsNames.COURSE_ID); String feedbackSessionName = getNonNullRequestParamValue(Const.ParamsNames.FEEDBACK_SESSION_NAME); Intent intent = Intent.valueOf(getNonNullRequestParamValue(Const.ParamsNames.INTENT)); - + if (isCourseMigrated(courseId)) { checkSpecificAccessControlSql(courseId, feedbackSessionName, intent); } else { @@ -38,7 +38,8 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException { } } - private void checkSpecificAccessControlDatastore(String courseId, String feedbackSessionName, Intent intent) throws UnauthorizedAccessException { + private void checkSpecificAccessControlDatastore( + String courseId, String feedbackSessionName, Intent intent) throws UnauthorizedAccessException { FeedbackSessionAttributes feedbackSession = getNonNullFeedbackSession(feedbackSessionName, courseId); switch (intent) { @@ -69,9 +70,10 @@ private void checkSpecificAccessControlDatastore(String courseId, String feedbac } } - private void checkSpecificAccessControlSql(String courseId, String feedbackSessionName, Intent intent) throws UnauthorizedAccessException { + private void checkSpecificAccessControlSql( + String courseId, String feedbackSessionName, Intent intent) throws UnauthorizedAccessException { FeedbackSession feedbackSession = getNonNullSqlFeedbackSession(feedbackSessionName, courseId); - + switch (intent) { case FULL_DETAIL: gateKeeper.verifyLoggedInUserPrivileges(userInfo); @@ -112,7 +114,7 @@ public JsonResult execute() { getRequestParamValue(Const.ParamsNames.FEEDBACK_RESULTS_SECTION_BY_GIVER_RECEIVER)); Intent intent = Intent.valueOf(getNonNullRequestParamValue(Const.ParamsNames.INTENT)); - + if (isCourseMigrated(courseId)) { return executeWithSql(courseId, feedbackSessionName, questionId, selectedSection, fetchType, intent); } else { @@ -120,7 +122,9 @@ public JsonResult execute() { } } - private JsonResult executeWithDatastore(String courseId, String feedbackSessionName, String questionId, String selectedSection, FeedbackResultFetchType fetchType, Intent intent) { + private JsonResult executeWithDatastore( + String courseId, String feedbackSessionName, String questionId, String selectedSection, + FeedbackResultFetchType fetchType, Intent intent) { InstructorAttributes instructor; StudentAttributes student; SessionResultsBundle bundle; @@ -160,7 +164,9 @@ private JsonResult executeWithDatastore(String courseId, String feedbackSessionN } } - private JsonResult executeWithSql(String courseId, String feedbackSessionName, String questionId, String selectedSection, FeedbackResultFetchType fetchType, Intent intent) { + private JsonResult executeWithSql( + String courseId, String feedbackSessionName, String questionId, String selectedSection, + FeedbackResultFetchType fetchType, Intent intent) { Instructor instructor; Student student; FeedbackSession feedbackSession = getNonNullSqlFeedbackSession(feedbackSessionName, courseId); From 6573955fcadbcad6170fdc0cb42bd5789c627946 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Sat, 17 Feb 2024 00:48:37 +0800 Subject: [PATCH 12/42] Migrate DB logic --- .../sqlapi/FeedbackResponseCommentsDb.java | 105 ++++++++++++- .../storage/sqlapi/FeedbackResponsesDb.java | 144 +++++++++++++++++- 2 files changed, 247 insertions(+), 2 deletions(-) diff --git a/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java b/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java index 9aff2b7251d..cbc795a8f3e 100644 --- a/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java +++ b/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java @@ -13,11 +13,12 @@ import teammates.storage.sqlentity.FeedbackResponse; import teammates.storage.sqlentity.FeedbackResponseComment; import teammates.storage.sqlentity.FeedbackSession; - +import teammates.storage.sqlentity.Section; import jakarta.persistence.criteria.CriteriaBuilder; import jakarta.persistence.criteria.CriteriaQuery; import jakarta.persistence.criteria.Join; import jakarta.persistence.criteria.Root; +import jakarta.persistence.criteria.Subquery; /** * Handles CRUD operations for feedbackResponseComments. @@ -196,4 +197,106 @@ public void updateFeedbackResponseComment(FeedbackResponseComment feedbackRespon merge(feedbackResponseComment); } + /** + * Gets all comments in a feedback session of a course. + */ + public List getFeedbackResponseCommentsForSession( + String courseId, String feedbackSessionName) { + assert courseId != null; + assert feedbackSessionName != null; + + CriteriaBuilder cb = HibernateUtil.getCriteriaBuilder(); + CriteriaQuery cq = cb.createQuery(FeedbackResponseComment.class); + Root root = cq.from(FeedbackResponseComment.class); + Join frJoin = root.join("feedbackResponse"); + Join fqJoin = frJoin.join("feedbackQuestion"); + Join fsJoin = fqJoin.join("feedbackSession"); + + cq.select(root) + .where(cb.and( + cb.equal(fsJoin.get("courseId"), courseId), + cb.equal(fsJoin.get("name"), feedbackSessionName) + )); + + return HibernateUtil.createQuery(cq).getResultList(); + } + + /** + * Gets all comments of a feedback question of a course. + */ + public List getFeedbackResponseCommentsForQuestion(UUID questionId) { + assert questionId != null; + + CriteriaBuilder cb = HibernateUtil.getCriteriaBuilder(); + CriteriaQuery cq = cb.createQuery(FeedbackResponseComment.class); + Root root = cq.from(FeedbackResponseComment.class); + Join frJoin = root.join("feedbackResponse"); + Join fqJoin = frJoin.join("feedbackQuestion"); + + cq.select(root) + .where(cb.and( + cb.equal(fqJoin.get("id"), questionId))); + + return HibernateUtil.createQuery(cq).getResultList(); + } + + /** + * Gets all comments which have its corresponding response given to/from a section of a feedback session of a course. + */ + public List getFeedbackResponseCommentsForSessionInSection( + String courseId, String feedbackSessionName, String sectionName) { + assert courseId != null; + assert feedbackSessionName != null; + assert sectionName != null; + + CriteriaBuilder cb = HibernateUtil.getCriteriaBuilder(); + CriteriaQuery cq = cb.createQuery(FeedbackResponseComment.class); + Root root = cq.from(FeedbackResponseComment.class); + Join frJoin = root.join("feedbackResponse"); + Join fqJoin = frJoin.join("feedbackQuestion"); + Join fsJoin = fqJoin.join("feedbackSession"); + Join cJoin = fsJoin.join("course"); + + Subquery

sq = cq.subquery(Section.class); + Root
sqRoot = sq.from(Section.class); + sq.select(sqRoot).where(cb.equal(sqRoot.get("name"), sectionName)); + + cq.select(root) + .where(cb.and( + cb.equal(cJoin.get("id"), courseId), + cb.equal(fsJoin.get("name"), feedbackSessionName), + cb.in(cJoin.get("sections")).value(sq) + )); + + return HibernateUtil.createQuery(cq).getResultList(); + } + + /** + * Gets all comments which have its corresponding response given to/from a section of a feedback question of a course. + */ + public List getFeedbackResponseCommentsForQuestionInSection( + UUID questionId, String sectionName) { + assert questionId != null; + assert sectionName != null; + + CriteriaBuilder cb = HibernateUtil.getCriteriaBuilder(); + CriteriaQuery cq = cb.createQuery(FeedbackResponseComment.class); + Root root = cq.from(FeedbackResponseComment.class); + Join fqJoin = root.join("feedbackQuestion"); + Join fsJoin = fqJoin.join("feedbackSession"); + Join cJoin = fsJoin.join("course"); + + Subquery
sq = cq.subquery(Section.class); + Root
sqRoot = sq.from(Section.class); + sq.select(sqRoot).where(cb.equal(sqRoot.get("name"), sectionName)); + + cq.select(root) + .where(cb.and( + cb.equal(fqJoin.get("id"), questionId), + cb.in(cJoin.get("sections")).value(sq) + )); + + return HibernateUtil.createQuery(cq).getResultList(); + } + } diff --git a/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java b/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java index e5b26e93f0d..ee9d40482a4 100644 --- a/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java +++ b/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java @@ -3,9 +3,11 @@ import static teammates.common.util.Const.ERROR_CREATE_ENTITY_ALREADY_EXISTS; import static teammates.common.util.Const.ERROR_UPDATE_NON_EXISTENT; +import java.util.ArrayList; import java.util.List; import java.util.UUID; +import teammates.common.datatransfer.FeedbackResultFetchType; import teammates.common.exception.EntityAlreadyExistsException; import teammates.common.exception.EntityDoesNotExistException; import teammates.common.exception.InvalidParametersException; @@ -14,11 +16,14 @@ import teammates.storage.sqlentity.FeedbackQuestion; import teammates.storage.sqlentity.FeedbackResponse; import teammates.storage.sqlentity.FeedbackSession; - +import teammates.storage.sqlentity.Team; +import teammates.storage.sqlentity.User; import jakarta.persistence.criteria.CriteriaBuilder; import jakarta.persistence.criteria.CriteriaDelete; import jakarta.persistence.criteria.CriteriaQuery; +import jakarta.persistence.criteria.Expression; import jakarta.persistence.criteria.Join; +import jakarta.persistence.criteria.Predicate; import jakarta.persistence.criteria.Root; import jakarta.persistence.criteria.Subquery; @@ -221,7 +226,144 @@ public FeedbackResponse updateFeedbackResponse(FeedbackResponse feedbackResponse } return merge(feedbackResponse); + } + + /** + * Gets all responses received by a user for a question. + */ + public List getFeedbackResponsesForRecipientForQuestion( + UUID questionId, String recipient) { + assert questionId != null; + assert recipient != null; + + CriteriaBuilder cb = HibernateUtil.getCriteriaBuilder(); + CriteriaQuery cq = cb.createQuery(FeedbackResponse.class); + Root root = cq.from(FeedbackResponse.class); + + cq.select(root) + .where(cb.and( + cb.equal(root.get("questionId"), questionId), + cb.equal(root.get("recipient"), recipient) + )); + + return HibernateUtil.createQuery(cq).getResultList(); + } + + + /** + * Gets all feedback responses for a question. + */ + public List getFeedbackResponsesForQuestion(UUID questionId) { + assert questionId != null; + + CriteriaBuilder cb = HibernateUtil.getCriteriaBuilder(); + CriteriaQuery cq = cb.createQuery(FeedbackResponse.class); + Root root = cq.from(FeedbackResponse.class); + + cq.select(root) + .where(cb.and( + cb.equal(root.get("questionId"), questionId) + )); + + return HibernateUtil.createQuery(cq).getResultList(); + } + + /** + * Gets all responses given to/from a section in a feedback session in a course. + * Optionally, retrieves by either giver, receiver sections, or both. + */ + public List getFeedbackResponsesForSessionInSection( + FeedbackSession feedbackSession, String courseId, String sectionId, FeedbackResultFetchType fetchType) { + assert feedbackSession != null; + assert courseId != null; + assert sectionId != null; + assert fetchType != null; + + CriteriaBuilder cb = HibernateUtil.getCriteriaBuilder(); + CriteriaQuery cq = cb.createQuery(FeedbackResponse.class); + Root root = cq.from(FeedbackResponse.class); + Join fqJoin = root.join("feedbackQuestion"); + Join fsJoin = fqJoin.join("feedbackSession"); + + // unless specified by fetchType, do not filter by giver/recipient section + Predicate giverSectionFilter = cb.isTrue(cb.literal(true)); + Predicate recipientSectionFilter = cb.isTrue(cb.literal(true)); + if (fetchType.shouldFetchByGiver()) { + giverSectionFilter = cb.equal(root.get("giverSectionId"), sectionId); + } + if (fetchType.shouldFetchByReceiver()) { + giverSectionFilter = cb.equal(root.get("recipientSectionId"), sectionId); + } + + cq.select(root) + .where(cb.and( + cb.equal(fqJoin.get("sessionId"), feedbackSession.getId()), + cb.equal(fsJoin.get("courseId"), courseId), + giverSectionFilter, + recipientSectionFilter + )); + + return HibernateUtil.createQuery(cq).getResultList(); + } + + /** + * Gets all feedback responses of a question in a specific section. + */ + public List getFeedbackResponsesForQuestionInSection( + UUID questionId, String sectionId, FeedbackResultFetchType fetchType) { + assert questionId != null; + assert sectionId != null; + assert fetchType != null; + + CriteriaBuilder cb = HibernateUtil.getCriteriaBuilder(); + CriteriaQuery cq = cb.createQuery(FeedbackResponse.class); + Root root = cq.from(FeedbackResponse.class); + Join fqJoin = root.join("feedbackQuestion"); + + // unless specified by fetchType, do not filter by giver/recipient section + Predicate giverSectionFilter = cb.isTrue(cb.literal(true)); + Predicate recipientSectionFilter = cb.isTrue(cb.literal(true)); + + if (fetchType.shouldFetchByGiver()) { + giverSectionFilter = cb.equal(root.get("giverSectionId"), sectionId); + } + if (fetchType.shouldFetchByReceiver()) { + giverSectionFilter = cb.equal(root.get("recipientSectionId"), sectionId); + } + + cq.select(root) + .where(cb.and( + cb.equal(fqJoin.get("id"), questionId), + giverSectionFilter, + recipientSectionFilter + )); + + return HibernateUtil.createQuery(cq).getResultList(); + + } + + /** + * Gets all responses of a feedback session in a course. + */ + public List getFeedbackResponsesForSession( + FeedbackSession feedbackSession, String courseId) { + assert feedbackSession != null; + assert courseId != null; + + CriteriaBuilder cb = HibernateUtil.getCriteriaBuilder(); + CriteriaQuery cq = cb.createQuery(FeedbackResponse.class); + Root root = cq.from(FeedbackResponse.class); + Join fqJoin = root.join("feedbackQuestion"); + Join fsJoin = fqJoin.join("feedbackSession"); + + cq.select(root) + .where(cb.and( + cb.equal(fsJoin.get("id"), feedbackSession.getId()), + cb.equal(fsJoin.get("courseId"), courseId) + )); + + return HibernateUtil.createQuery(cq).getResultList(); } } From 80d651e6136eb268669aca3ca6972d9b336c1487 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Mon, 19 Feb 2024 12:04:18 +0800 Subject: [PATCH 13/42] Fix checkstyle errors --- .../storage/sqlapi/FeedbackResponseCommentsDb.java | 1 + .../teammates/storage/sqlapi/FeedbackResponsesDb.java | 11 +++-------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java b/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java index c94ab015e2e..4eeeb694458 100644 --- a/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java +++ b/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java @@ -14,6 +14,7 @@ import teammates.storage.sqlentity.FeedbackResponseComment; import teammates.storage.sqlentity.FeedbackSession; import teammates.storage.sqlentity.Section; + import jakarta.persistence.criteria.CriteriaBuilder; import jakarta.persistence.criteria.CriteriaQuery; import jakarta.persistence.criteria.Join; diff --git a/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java b/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java index ee9d40482a4..c73dcce3c55 100644 --- a/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java +++ b/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java @@ -3,7 +3,6 @@ import static teammates.common.util.Const.ERROR_CREATE_ENTITY_ALREADY_EXISTS; import static teammates.common.util.Const.ERROR_UPDATE_NON_EXISTENT; -import java.util.ArrayList; import java.util.List; import java.util.UUID; @@ -16,12 +15,10 @@ import teammates.storage.sqlentity.FeedbackQuestion; import teammates.storage.sqlentity.FeedbackResponse; import teammates.storage.sqlentity.FeedbackSession; -import teammates.storage.sqlentity.Team; -import teammates.storage.sqlentity.User; + import jakarta.persistence.criteria.CriteriaBuilder; import jakarta.persistence.criteria.CriteriaDelete; import jakarta.persistence.criteria.CriteriaQuery; -import jakarta.persistence.criteria.Expression; import jakarta.persistence.criteria.Join; import jakarta.persistence.criteria.Predicate; import jakarta.persistence.criteria.Root; @@ -249,7 +246,6 @@ public List getFeedbackResponsesForRecipientForQuestion( return HibernateUtil.createQuery(cq).getResultList(); } - /** * Gets all feedback responses for a question. */ @@ -303,7 +299,7 @@ public List getFeedbackResponsesForSessionInSection( giverSectionFilter, recipientSectionFilter )); - + return HibernateUtil.createQuery(cq).getResultList(); } @@ -338,9 +334,8 @@ public List getFeedbackResponsesForQuestionInSection( giverSectionFilter, recipientSectionFilter )); - - return HibernateUtil.createQuery(cq).getResultList(); + return HibernateUtil.createQuery(cq).getResultList(); } /** From 7e960f7442f486f829e2938a65efe4e51d9ec1bc Mon Sep 17 00:00:00 2001 From: Xenos F Date: Mon, 19 Feb 2024 12:15:16 +0800 Subject: [PATCH 14/42] Move default instructor team entity to const --- .../java/teammates/common/datatransfer/SqlCourseRoster.java | 2 +- src/main/java/teammates/common/util/Const.java | 2 ++ src/main/java/teammates/storage/sqlentity/Instructor.java | 4 ---- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/main/java/teammates/common/datatransfer/SqlCourseRoster.java b/src/main/java/teammates/common/datatransfer/SqlCourseRoster.java index dd43e5876e5..83b8021cfb2 100644 --- a/src/main/java/teammates/common/datatransfer/SqlCourseRoster.java +++ b/src/main/java/teammates/common/datatransfer/SqlCourseRoster.java @@ -144,7 +144,7 @@ public ParticipantInfo getInfoForIdentifier(String identifier) { Instructor instructor = getInstructorForEmail(identifier); name = instructor.getName(); - team = instructor.getTeam(); + team = Const.USER_TEAM_ENTITY_FOR_INSTRUCTOR; section = Const.DEFAULT_SECTION_ENTITY; } else if (isTeam) { Student teamMember = getTeamToMembersTable().get(identifier).iterator().next(); diff --git a/src/main/java/teammates/common/util/Const.java b/src/main/java/teammates/common/util/Const.java index 78e0ba0df35..2792a67fc60 100644 --- a/src/main/java/teammates/common/util/Const.java +++ b/src/main/java/teammates/common/util/Const.java @@ -6,6 +6,7 @@ import java.time.Instant; import teammates.storage.sqlentity.Section; +import teammates.storage.sqlentity.Team; /** * Stores constants that are widely used across classes. @@ -19,6 +20,7 @@ public final class Const { public static final String USER_NOBODY_TEXT = "-"; public static final String USER_TEAM_FOR_INSTRUCTOR = "Instructors"; + public static final Team USER_TEAM_ENTITY_FOR_INSTRUCTOR = new Team(null, Const.USER_TEAM_FOR_INSTRUCTOR); public static final String DEFAULT_DISPLAY_NAME_FOR_INSTRUCTOR = "Instructor"; diff --git a/src/main/java/teammates/storage/sqlentity/Instructor.java b/src/main/java/teammates/storage/sqlentity/Instructor.java index 24821a29f61..6848d80ac4b 100644 --- a/src/main/java/teammates/storage/sqlentity/Instructor.java +++ b/src/main/java/teammates/storage/sqlentity/Instructor.java @@ -25,9 +25,6 @@ @Entity @Table(name = "Instructors") public class Instructor extends User { - - private static Team userTeam = new Team(null, Const.USER_TEAM_FOR_INSTRUCTOR); - @Column(nullable = false) private boolean isDisplayedToStudents; @@ -53,7 +50,6 @@ public Instructor(Course course, String name, String email, boolean isDisplayedT this.setDisplayName(displayName); this.setRole(role); this.setPrivileges(privileges); - this.setTeam(userTeam); } public boolean isDisplayedToStudents() { From 901307a1b1c6d7e66840d400a69f2023be466637 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Mon, 19 Feb 2024 13:01:15 +0800 Subject: [PATCH 15/42] Add test for SqlSessionResultsBundle --- .../SqlSessionResultsBundleTest.java | 178 ++++++++++++++++++ .../java/teammates/test/BaseTestCase.java | 12 ++ 2 files changed, 190 insertions(+) create mode 100644 src/test/java/teammates/common/datatransfer/SqlSessionResultsBundleTest.java diff --git a/src/test/java/teammates/common/datatransfer/SqlSessionResultsBundleTest.java b/src/test/java/teammates/common/datatransfer/SqlSessionResultsBundleTest.java new file mode 100644 index 00000000000..12a873cea55 --- /dev/null +++ b/src/test/java/teammates/common/datatransfer/SqlSessionResultsBundleTest.java @@ -0,0 +1,178 @@ +package teammates.common.datatransfer; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.testng.annotations.Test; + +import teammates.common.util.Const; +import teammates.storage.sqlentity.FeedbackQuestion; +import teammates.storage.sqlentity.FeedbackResponse; +import teammates.storage.sqlentity.FeedbackSession; +import teammates.test.BaseTestCase; + +/** + * SUT: {@link SqlSessionResultsBundle}. + */ +public class SqlSessionResultsBundleTest extends BaseTestCase { + + @Test + public void testGetQuestionResponseMap() { + SqlDataBundle responseBundle = loadSqlDataBundle("/FeedbackSessionResultsBundleTest.json"); + + List allExpectedResponses = new ArrayList<>(); + allExpectedResponses.add(responseBundle.feedbackResponses.get("response1ForQ1S1C1").toString()); + allExpectedResponses.add(responseBundle.feedbackResponses.get("response2ForQ1S1C1").toString()); + + SqlSessionResultsBundle bundle = + new SqlSessionResultsBundle( + new ArrayList<>(responseBundle.feedbackQuestions.values()), + new ArrayList<>(responseBundle.feedbackResponses.values()), + new ArrayList<>(), + new HashMap<>(), + new HashMap<>(), + new HashMap<>(), + new HashMap<>(), + new SqlCourseRoster(new ArrayList<>(responseBundle.students.values()), + new ArrayList<>(responseBundle.instructors.values())) + ); + + ______TS("Test question having responses"); + FeedbackQuestion fq = responseBundle.feedbackQuestions.get("qn1InSession1InCourse1"); + List allResponses = bundle.getQuestionResponseMap().get(fq); + assertEquals(2, allResponses.size()); + List allResponsesString = new ArrayList<>(); + allResponsesString.add(allResponses.get(0).toString()); + allResponsesString.add(allResponses.get(1).toString()); + assertEquals(allExpectedResponses, allResponsesString); + + ______TS("Test question having no responses"); + fq = responseBundle.feedbackQuestions.get("qn3InSession1InCourse1"); + allResponses = bundle.getQuestionResponseMap().get(fq); + assertEquals(0, allResponses.size()); + } + + @Test + public void testGetQuestionMissingResponseMap() { + SqlDataBundle responseBundle = loadSqlDataBundle("/FeedbackSessionResultsBundleTest.json"); + + List expectedMissingResponses = new ArrayList<>(); + expectedMissingResponses.add(responseBundle.feedbackResponses.get("response1ForQ1S1C1").toString()); + expectedMissingResponses.add(responseBundle.feedbackResponses.get("response2ForQ1S1C1").toString()); + + SqlSessionResultsBundle bundle = + new SqlSessionResultsBundle( + new ArrayList<>(responseBundle.feedbackQuestions.values()), + new ArrayList<>(responseBundle.feedbackResponses.values()), + new ArrayList<>(), + new HashMap<>(), + new HashMap<>(), + new HashMap<>(), + new HashMap<>(), + new SqlCourseRoster(new ArrayList<>(responseBundle.students.values()), + new ArrayList<>(responseBundle.instructors.values())) + ); + + ______TS("Test question having missing responses"); + FeedbackQuestion fq = responseBundle.feedbackQuestions.get("qn1InSession1InCourse1"); + List missingResponses = bundle.getQuestionMissingResponseMap().get(fq); + assertEquals(2, missingResponses.size()); + List missingResponsesString = new ArrayList<>(); + missingResponsesString.add(missingResponses.get(0).toString()); + missingResponsesString.add(missingResponses.get(1).toString()); + assertEquals(expectedMissingResponses, missingResponsesString); + + ______TS("Test question having no missing responses"); + fq = responseBundle.feedbackQuestions.get("qn3InSession1InCourse1"); + missingResponses = bundle.getQuestionMissingResponseMap().get(fq); + assertEquals(0, missingResponses.size()); + } + + @Test + public void testIsResponseGiverRecipientVisible_typicalCase_shouldReturnCorrectValues() { + + SqlDataBundle responseBundle = loadSqlDataBundle("/FeedbackSessionResultsBundleTest.json"); + + FeedbackSession session1Course1 = getTypicalFeedbackSessionForCourse(getTypicalCourse()); + + FeedbackQuestion question1ForS1C1 = getTypicalFeedbackQuestionForSession(session1Course1); + FeedbackQuestion question2ForS1C1 = getTypicalFeedbackQuestionForSession(session1Course1); + + FeedbackResponse response1ForQ1S1C1 = getTypicalFeedbackResponseForQuestion(question1ForS1C1); + FeedbackResponse response2ForQ1S1C1 = getTypicalFeedbackResponseForQuestion(question1ForS1C1); + FeedbackResponse response1ForQ2S1C1 = getTypicalFeedbackResponseForQuestion(question2ForS1C1); + FeedbackResponse response2ForQ2S1C1 = getTypicalFeedbackResponseForQuestion(question2ForS1C1); + + Map responseGiverVisibilityTable = new HashMap<>(); + responseGiverVisibilityTable.put(response1ForQ1S1C1, true); + responseGiverVisibilityTable.put(response2ForQ1S1C1, false); + responseGiverVisibilityTable.put(response1ForQ2S1C1, true); + responseGiverVisibilityTable.put(response2ForQ2S1C1, false); + + Map responseRecipientVisibilityTable = new HashMap<>(); + responseRecipientVisibilityTable.put(response1ForQ1S1C1, false); + responseRecipientVisibilityTable.put(response2ForQ1S1C1, true); + responseRecipientVisibilityTable.put(response1ForQ2S1C1, true); + responseRecipientVisibilityTable.put(response2ForQ2S1C1, false); + + SqlSessionResultsBundle bundle = + new SqlSessionResultsBundle( + new ArrayList<>(responseBundle.feedbackQuestions.values()), + new ArrayList<>(responseBundle.feedbackResponses.values()), + new ArrayList<>(), + responseGiverVisibilityTable, + responseRecipientVisibilityTable, + new HashMap<>(), + new HashMap<>(), + new SqlCourseRoster(new ArrayList<>(responseBundle.students.values()), + new ArrayList<>(responseBundle.instructors.values())) + ); + + for (Map.Entry visibilityEntry : responseGiverVisibilityTable.entrySet()) { + assertEquals(visibilityEntry.getValue(), + bundle.isResponseGiverVisible(visibilityEntry.getKey())); + } + + for (Map.Entry visibilityEntry : responseRecipientVisibilityTable.entrySet()) { + assertEquals(visibilityEntry.getValue(), + bundle.isResponseRecipientVisible(visibilityEntry.getKey())); + } + } + + @Test + public void testIsCommentGiverVisible_typicalCase_shouldReturnCorrectValues() { + + SqlDataBundle responseBundle = loadSqlDataBundle("/FeedbackSessionResultsBundleTest.json"); + + Map commentGiverVisibilityTable = new HashMap<>(); + commentGiverVisibilityTable.put(1L, true); + commentGiverVisibilityTable.put(2L, false); + + SqlSessionResultsBundle bundle = + new SqlSessionResultsBundle( + new ArrayList<>(responseBundle.feedbackQuestions.values()), + new ArrayList<>(responseBundle.feedbackResponses.values()), + new ArrayList<>(), + new HashMap<>(), + new HashMap<>(), + new HashMap<>(), + commentGiverVisibilityTable, + new SqlCourseRoster(new ArrayList<>(responseBundle.students.values()), + new ArrayList<>(responseBundle.instructors.values())) + ); + + assertTrue(bundle.isCommentGiverVisible(responseBundle.feedbackResponseComments.get("comment1FromT1C1ToR1Q1S1C1"))); + assertFalse(bundle.isCommentGiverVisible(responseBundle.feedbackResponseComments.get("comment2FromT1C1ToR1Q1S1C1"))); + } + + @Test + public void testGetAnonName_typicalCase_shouldGenerateCorrectly() { + String anonName = SqlSessionResultsBundle.getAnonName(FeedbackParticipantType.STUDENTS, ""); + assertTrue(anonName.startsWith(Const.DISPLAYED_NAME_FOR_ANONYMOUS_PARTICIPANT)); + + anonName = SqlSessionResultsBundle.getAnonName(FeedbackParticipantType.STUDENTS, "test@gmail.com"); + assertTrue(anonName.startsWith(Const.DISPLAYED_NAME_FOR_ANONYMOUS_PARTICIPANT)); + } +} diff --git a/src/test/java/teammates/test/BaseTestCase.java b/src/test/java/teammates/test/BaseTestCase.java index e22c1ea1b66..1635e8ca601 100644 --- a/src/test/java/teammates/test/BaseTestCase.java +++ b/src/test/java/teammates/test/BaseTestCase.java @@ -19,7 +19,9 @@ import teammates.common.datatransfer.NotificationStyle; import teammates.common.datatransfer.NotificationTargetUser; import teammates.common.datatransfer.SqlDataBundle; +import teammates.common.datatransfer.questions.FeedbackResponseDetails; import teammates.common.datatransfer.questions.FeedbackTextQuestionDetails; +import teammates.common.datatransfer.questions.FeedbackTextResponseDetails; import teammates.common.util.Const; import teammates.common.util.FieldValidator; import teammates.common.util.JsonUtils; @@ -27,6 +29,7 @@ import teammates.storage.sqlentity.Account; import teammates.storage.sqlentity.Course; import teammates.storage.sqlentity.FeedbackQuestion; +import teammates.storage.sqlentity.FeedbackResponse; import teammates.storage.sqlentity.FeedbackSession; import teammates.storage.sqlentity.Instructor; import teammates.storage.sqlentity.Notification; @@ -170,6 +173,15 @@ protected FeedbackQuestion getTypicalFeedbackQuestionForSession(FeedbackSession new FeedbackTextQuestionDetails("test question text")); } + protected FeedbackResponse getTypicalFeedbackResponseForQuestion(FeedbackQuestion question) { + return FeedbackResponse.makeResponse(question, "test-giver", getTypicalSection(), "test-recipient", + getTypicalSection(), getTypicalFeedbackResponseDetails()); + } + + protected FeedbackResponseDetails getTypicalFeedbackResponseDetails() { + return new FeedbackTextResponseDetails(); + } + /** * Populates the feedback question and response IDs within the data bundle. * From 29900bd3226ebe42df444bbeb3e40612573eacf1 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Tue, 20 Feb 2024 16:51:19 +0800 Subject: [PATCH 16/42] Fix SQL results bundle test --- .../datatransfer/SqlSessionResultsBundle.java | 2 +- .../SqlSessionResultsBundleTest.java | 34 +- .../SqlFeedbackSessionResultsBundleTest.json | 1339 +++++++++++++++++ 3 files changed, 1360 insertions(+), 15 deletions(-) create mode 100644 src/test/resources/data/SqlFeedbackSessionResultsBundleTest.json diff --git a/src/main/java/teammates/common/datatransfer/SqlSessionResultsBundle.java b/src/main/java/teammates/common/datatransfer/SqlSessionResultsBundle.java index e4693818876..9521b4b94d6 100644 --- a/src/main/java/teammates/common/datatransfer/SqlSessionResultsBundle.java +++ b/src/main/java/teammates/common/datatransfer/SqlSessionResultsBundle.java @@ -53,7 +53,7 @@ private Map> buildQuestionToResponseMap } for (FeedbackResponse response : responses) { FeedbackQuestion question = response.getFeedbackQuestion(); - List responsesForQuestion = questionToResponseMap.get(question.getId()); + List responsesForQuestion = questionToResponseMap.get(question); responsesForQuestion.add(response); } return questionToResponseMap; diff --git a/src/test/java/teammates/common/datatransfer/SqlSessionResultsBundleTest.java b/src/test/java/teammates/common/datatransfer/SqlSessionResultsBundleTest.java index 12a873cea55..af600b9fd76 100644 --- a/src/test/java/teammates/common/datatransfer/SqlSessionResultsBundleTest.java +++ b/src/test/java/teammates/common/datatransfer/SqlSessionResultsBundleTest.java @@ -10,6 +10,7 @@ import teammates.common.util.Const; import teammates.storage.sqlentity.FeedbackQuestion; import teammates.storage.sqlentity.FeedbackResponse; +import teammates.storage.sqlentity.FeedbackResponseComment; import teammates.storage.sqlentity.FeedbackSession; import teammates.test.BaseTestCase; @@ -20,11 +21,11 @@ public class SqlSessionResultsBundleTest extends BaseTestCase { @Test public void testGetQuestionResponseMap() { - SqlDataBundle responseBundle = loadSqlDataBundle("/FeedbackSessionResultsBundleTest.json"); + SqlDataBundle responseBundle = loadSqlDataBundle("/SqlFeedbackSessionResultsBundleTest.json"); List allExpectedResponses = new ArrayList<>(); - allExpectedResponses.add(responseBundle.feedbackResponses.get("response1ForQ1S1C1").toString()); - allExpectedResponses.add(responseBundle.feedbackResponses.get("response2ForQ1S1C1").toString()); + allExpectedResponses.add(responseBundle.feedbackResponses.get("response1ForQ1").toString()); + allExpectedResponses.add(responseBundle.feedbackResponses.get("response2ForQ1").toString()); SqlSessionResultsBundle bundle = new SqlSessionResultsBundle( @@ -49,24 +50,24 @@ public void testGetQuestionResponseMap() { assertEquals(allExpectedResponses, allResponsesString); ______TS("Test question having no responses"); - fq = responseBundle.feedbackQuestions.get("qn3InSession1InCourse1"); + fq = responseBundle.feedbackQuestions.get("qn4InSession1InCourse1"); allResponses = bundle.getQuestionResponseMap().get(fq); assertEquals(0, allResponses.size()); } @Test public void testGetQuestionMissingResponseMap() { - SqlDataBundle responseBundle = loadSqlDataBundle("/FeedbackSessionResultsBundleTest.json"); + SqlDataBundle responseBundle = loadSqlDataBundle("/SqlFeedbackSessionResultsBundleTest.json"); List expectedMissingResponses = new ArrayList<>(); - expectedMissingResponses.add(responseBundle.feedbackResponses.get("response1ForQ1S1C1").toString()); - expectedMissingResponses.add(responseBundle.feedbackResponses.get("response2ForQ1S1C1").toString()); + expectedMissingResponses.add(responseBundle.feedbackResponses.get("response1ForQ1").toString()); + expectedMissingResponses.add(responseBundle.feedbackResponses.get("response2ForQ1").toString()); SqlSessionResultsBundle bundle = new SqlSessionResultsBundle( new ArrayList<>(responseBundle.feedbackQuestions.values()), - new ArrayList<>(responseBundle.feedbackResponses.values()), new ArrayList<>(), + new ArrayList<>(responseBundle.feedbackResponses.values()), new HashMap<>(), new HashMap<>(), new HashMap<>(), @@ -85,7 +86,7 @@ public void testGetQuestionMissingResponseMap() { assertEquals(expectedMissingResponses, missingResponsesString); ______TS("Test question having no missing responses"); - fq = responseBundle.feedbackQuestions.get("qn3InSession1InCourse1"); + fq = responseBundle.feedbackQuestions.get("qn4InSession1InCourse1"); missingResponses = bundle.getQuestionMissingResponseMap().get(fq); assertEquals(0, missingResponses.size()); } @@ -93,7 +94,7 @@ public void testGetQuestionMissingResponseMap() { @Test public void testIsResponseGiverRecipientVisible_typicalCase_shouldReturnCorrectValues() { - SqlDataBundle responseBundle = loadSqlDataBundle("/FeedbackSessionResultsBundleTest.json"); + SqlDataBundle responseBundle = loadSqlDataBundle("/SqlFeedbackSessionResultsBundleTest.json"); FeedbackSession session1Course1 = getTypicalFeedbackSessionForCourse(getTypicalCourse()); @@ -143,8 +144,7 @@ public void testIsResponseGiverRecipientVisible_typicalCase_shouldReturnCorrectV @Test public void testIsCommentGiverVisible_typicalCase_shouldReturnCorrectValues() { - - SqlDataBundle responseBundle = loadSqlDataBundle("/FeedbackSessionResultsBundleTest.json"); + SqlDataBundle responseBundle = loadSqlDataBundle("/SqlFeedbackSessionResultsBundleTest.json"); Map commentGiverVisibilityTable = new HashMap<>(); commentGiverVisibilityTable.put(1L, true); @@ -163,8 +163,14 @@ public void testIsCommentGiverVisible_typicalCase_shouldReturnCorrectValues() { new ArrayList<>(responseBundle.instructors.values())) ); - assertTrue(bundle.isCommentGiverVisible(responseBundle.feedbackResponseComments.get("comment1FromT1C1ToR1Q1S1C1"))); - assertFalse(bundle.isCommentGiverVisible(responseBundle.feedbackResponseComments.get("comment2FromT1C1ToR1Q1S1C1"))); + // Manually add comment IDs as loadSqlDataBundle does not add comment IDs + FeedbackResponseComment comment1 = responseBundle.feedbackResponseComments.get("comment1ToResponse1ForQ1"); + FeedbackResponseComment comment2 = responseBundle.feedbackResponseComments.get("comment2ToResponse1ForQ1"); + comment1.setId(1L); + comment2.setId(2L); + + assertTrue(bundle.isCommentGiverVisible(comment1)); + assertFalse(bundle.isCommentGiverVisible(comment2)); } @Test diff --git a/src/test/resources/data/SqlFeedbackSessionResultsBundleTest.json b/src/test/resources/data/SqlFeedbackSessionResultsBundleTest.json new file mode 100644 index 00000000000..6ded6ee8577 --- /dev/null +++ b/src/test/resources/data/SqlFeedbackSessionResultsBundleTest.json @@ -0,0 +1,1339 @@ +{ + "accounts": { + "instructor1": { + "id": "00000000-0000-4000-8000-000000000001", + "googleId": "instructor1", + "name": "Instructor 1", + "email": "instr1@teammates.tmt" + }, + "instructor2": { + "id": "00000000-0000-4000-8000-000000000002", + "googleId": "instructor2", + "name": "Instructor 2", + "email": "instr2@teammates.tmt" + }, + "instructorOfArchivedCourse": { + "id": "00000000-0000-4000-8000-000000000003", + "googleId": "instructorOfArchivedCourse", + "name": "Instructor Of Archived Course", + "email": "instructorOfArchivedCourse@archiveCourse.tmt" + }, + "instructorOfUnregisteredCourse": { + "id": "00000000-0000-4000-8000-000000000004", + "googleId": "InstructorOfUnregisteredCourse", + "name": "Instructor Of Unregistered Course", + "email": "instructorOfUnregisteredCourse@UnregisteredCourse.tmt" + }, + "instructorOfCourse2WithUniqueDisplayName": { + "id": "00000000-0000-4000-8000-000000000005", + "googleId": "instructorOfCourse2WithUniqueDisplayName", + "name": "Instructor Of Course 2 With Unique Display Name", + "email": "instructorOfCourse2WithUniqueDisplayName@teammates.tmt" + }, + "unregisteredInstructor1": { + "id": "00000000-0000-4000-8000-000000000006", + "googleId": "unregisteredInstructor1", + "name": "Unregistered Instructor 1", + "email": "unregisteredinstructor1@gmail.tmt" + }, + "unregisteredInstructor2": { + "id": "00000000-0000-4000-8000-000000000007", + "googleId": "unregisteredInstructor2", + "name": "Unregistered Instructor 2", + "email": "unregisteredinstructor2@gmail.tmt" + }, + "student1": { + "id": "00000000-0000-4000-8000-000000000101", + "googleId": "idOfStudent1Course1", + "name": "Student 1", + "email": "student1@teammates.tmt" + }, + "student2": { + "id": "00000000-0000-4000-8000-000000000102", + "googleId": "idOfStudent2Course1", + "name": "Student 2", + "email": "student2@teammates.tmt" + }, + "student3": { + "id": "00000000-0000-4000-8000-000000000103", + "googleId": "idOfStudent3Course1", + "name": "Student 3", + "email": "student3@teammates.tmt" + } + }, + "accountRequests": { + "instructor1": { + "id": "00000000-0000-4000-8000-000000000101", + "name": "Instructor 1", + "email": "instr1@teammates.tmt", + "institute": "TEAMMATES Test Institute 1", + "registeredAt": "2010-02-14T00:00:00Z" + }, + "instructor2": { + "id": "00000000-0000-4000-8000-000000000102", + "name": "Instructor 2", + "email": "instr2@teammates.tmt", + "institute": "TEAMMATES Test Institute 1", + "registeredAt": "2015-02-14T00:00:00Z" + }, + "instructor3": { + "name": "Instructor 3 of CourseNoRegister", + "email": "instr3@teammates.tmt", + "institute": "TEAMMATES Test Institute 1", + "createdAt": "2011-01-01T00:00:00Z", + "registeredAt": "1970-02-14T00:00:00Z" + }, + "instructor1OfCourse1": { + "name": "Instructor 1 of Course 1", + "email": "instr1@course1.tmt", + "institute": "TEAMMATES Test Institute 1", + "createdAt": "2011-01-01T00:00:00Z", + "registeredAt": "1970-02-14T00:00:00Z" + }, + "instructor2OfCourse1": { + "name": "Instructor 2 of Course 1", + "email": "instr2@course1.tmt", + "institute": "TEAMMATES Test Institute 1", + "createdAt": "2011-01-01T00:00:00Z", + "registeredAt": "1970-02-14T00:00:00Z" + }, + "instructor1OfCourse2": { + "name": "Instructor 1 of Course 2", + "email": "instr1@course2.tmt", + "institute": "TEAMMATES Test Institute 1", + "createdAt": "2011-01-01T00:00:00Z", + "registeredAt": "1970-02-14T00:00:00Z" + }, + "instructor2OfCourse2": { + "name": "Instructor 2 of Course 2", + "email": "instr2@course2.tmt", + "institute": "TEAMMATES Test Institute 1", + "createdAt": "2011-01-01T00:00:00Z", + "registeredAt": "1970-02-14T00:00:00Z" + }, + "instructor1OfCourse3": { + "name": "Instructor 1 of Course 3", + "email": "instr1@course3.tmt", + "institute": "TEAMMATES Test Institute 1", + "createdAt": "2011-01-01T00:00:00Z", + "registeredAt": "1970-02-14T00:00:00Z" + }, + "instructor2OfCourse3": { + "name": "Instructor 2 of Course 3", + "email": "instr2@course3.tmt", + "institute": "TEAMMATES Test Institute 1", + "createdAt": "2011-01-01T00:00:00Z", + "registeredAt": "1970-02-14T00:00:00Z" + }, + "unregisteredInstructor1": { + "name": "Unregistered Instructor 1", + "email": "unregisteredinstructor1@gmail.tmt", + "institute": "TEAMMATES Test Institute 1", + "createdAt": "2011-01-01T00:00:00Z" + }, + "unregisteredInstructor2": { + "name": "Unregistered Instructor 2", + "email": "unregisteredinstructor2@gmail.tmt", + "institute": "TEAMMATES Test Institute 2", + "createdAt": "2011-01-01T00:00:00Z" + } + }, + "courses": { + "course1": { + "createdAt": "2012-04-01T23:59:00Z", + "id": "course-1", + "name": "Typical Course 1", + "institute": "TEAMMATES Test Institute 0", + "timeZone": "Africa/Johannesburg" + }, + "course2": { + "createdAt": "2012-04-01T23:59:00Z", + "id": "course-2", + "name": "Typical Course 2", + "institute": "TEAMMATES Test Institute 1", + "timeZone": "Asia/Singapore" + }, + "course3": { + "createdAt": "2012-04-01T23:59:00Z", + "id": "course-3", + "name": "Typical Course 3", + "institute": "TEAMMATES Test Institute 1", + "timeZone": "Asia/Singapore" + }, + "course4": { + "createdAt": "2012-04-01T23:59:00Z", + "id": "course-4", + "name": "Typical Course 4", + "institute": "TEAMMATES Test Institute 1", + "timeZone": "Asia/Singapore" + }, + "archivedCourse": { + "id": "archived-course", + "name": "Archived Course", + "institute": "TEAMMATES Test Institute 2", + "timeZone": "UTC" + }, + "unregisteredCourse": { + "id": "unregistered-course", + "name": "Unregistered Course", + "institute": "TEAMMATES Test Institute 3", + "timeZone": "UTC" + } + }, + "sections": { + "section1InCourse1": { + "id": "00000000-0000-4000-8000-000000000201", + "course": { + "id": "course-1" + }, + "name": "Section 1" + }, + "section1InCourse2": { + "id": "00000000-0000-4000-8000-000000000202", + "course": { + "id": "course-2" + }, + "name": "Section 2" + }, + "section2InCourse1": { + "id": "00000000-0000-4000-8000-000000000203", + "course": { + "id": "course-1" + }, + "name": "Section 3" + }, + "section1InCourse3": { + "id": "00000000-0000-4000-8000-000000000204", + "course": { + "id": "course-3" + }, + "name": "Section 1" + } + }, + "teams": { + "team1InCourse1": { + "id": "00000000-0000-4000-8000-000000000301", + "section": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "name": "Team 1" + }, + "team1InCourse2": { + "id": "00000000-0000-4000-8000-000000000302", + "section": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "name": "Team 2" + }, + "team2InCourse2": { + "id": "00000000-0000-4000-8000-000000000303", + "section": { + "id": "00000000-0000-4000-8000-000000000203" + }, + "name": "Team 3" + }, + "team1InCourse3": { + "id": "00000000-0000-4000-8000-000000000304", + "section": { + "id": "00000000-0000-4000-8000-000000000204" + }, + "name": "Team 1" + } + }, + "deadlineExtensions": { + "student1InCourse1Session1": { + "id": "00000000-0000-4000-8000-000000000401", + "user": { + "id": "00000000-0000-4000-8000-000000000601", + "type": "student" + }, + "feedbackSession": { + "id": "00000000-0000-4000-8000-000000000701" + }, + "endTime": "2027-04-30T23:00:00Z", + "isClosingSoonEmailSent": false + }, + "instructor1InCourse1Session1": { + "id": "00000000-0000-4000-8000-000000000402", + "user": { + "id": "00000000-0000-4000-8000-000000000501", + "type": "instructor" + }, + "feedbackSession": { + "id": "00000000-0000-4000-8000-000000000701" + }, + "endTime": "2027-04-30T23:00:00Z", + "isClosingSoonEmailSent": false + } + }, + "instructors": { + "instructor1OfCourse1": { + "id": "00000000-0000-4000-8000-000000000501", + "account": { + "id": "00000000-0000-4000-8000-000000000001" + }, + "course": { + "id": "course-1" + }, + "name": "Instructor 1", + "email": "instr1@teammates.tmt", + "role": "INSTRUCTOR_PERMISSION_ROLE_COOWNER", + "isDisplayedToStudents": true, + "displayName": "Instructor", + "privileges": { + "courseLevel": { + "canModifyCourse": true, + "canModifyInstructor": true, + "canModifySession": true, + "canModifyStudent": true, + "canViewStudentInSections": true, + "canViewSessionInSections": true, + "canSubmitSessionInSections": true, + "canModifySessionCommentsInSections": true + }, + "sectionLevel": {}, + "sessionLevel": {} + } + }, + "instructor2OfCourse1": { + "id": "00000000-0000-4000-8000-000000000502", + "account": { + "id": "00000000-0000-4000-8000-000000000002" + }, + "course": { + "id": "course-1" + }, + "name": "Instructor 2", + "email": "instr2@teammates.tmt", + "role": "INSTRUCTOR_PERMISSION_ROLE_TUTOR", + "isDisplayedToStudents": true, + "displayName": "Instructor", + "privileges": { + "courseLevel": { + "canModifyCourse": false, + "canModifyInstructor": false, + "canModifySession": false, + "canModifyStudent": false, + "canViewStudentInSections": true, + "canViewSessionInSections": true, + "canSubmitSessionInSections": true, + "canModifySessionCommentsInSections": false + }, + "sectionLevel": {}, + "sessionLevel": {} + } + }, + "instructorOfArchivedCourse": { + "id": "00000000-0000-4000-8000-000000000503", + "account": { + "id": "00000000-0000-4000-8000-000000000003" + }, + "course": { + "id": "archived-course" + }, + "name": "Instructor Of Archived Course", + "email": "instructorOfArchivedCourse@archiveCourse.tmt", + "isArchived": true, + "role": "INSTRUCTOR_PERMISSION_ROLE_COOWNER", + "isDisplayedToStudents": true, + "displayName": "Instructor", + "privileges": { + "courseLevel": { + "canModifyCourse": true, + "canModifyInstructor": true, + "canModifySession": true, + "canModifyStudent": true, + "canViewStudentInSections": true, + "canViewSessionInSections": true, + "canSubmitSessionInSections": true, + "canModifySessionCommentsInSections": false + }, + "sectionLevel": {}, + "sessionLevel": {} + } + }, + "instructorOfUnregisteredCourse": { + "id": "00000000-0000-4000-8000-000000000504", + "account": { + "id": "00000000-0000-4000-8000-000000000004" + }, + "course": { + "id": "unregistered-course" + }, + "name": "Instructor Of Unregistered Course", + "email": "instructorOfUnregisteredCourse@UnregisteredCourse.tmt", + "isArchived": false, + "role": "INSTRUCTOR_PERMISSION_ROLE_COOWNER", + "isDisplayedToStudents": true, + "displayName": "Instructor", + "privileges": { + "courseLevel": { + "canModifyCourse": true, + "canModifyInstructor": true, + "canModifySession": true, + "canModifyStudent": true, + "canViewStudentInSections": true, + "canViewSessionInSections": true, + "canSubmitSessionInSections": true, + "canModifySessionCommentsInSections": true + }, + "sectionLevel": {}, + "sessionLevel": {} + } + }, + "instructorOfCourse2WithUniqueDisplayName": { + "id": "00000000-0000-4000-8000-000000000505", + "account": { + "id": "00000000-0000-4000-8000-000000000005" + }, + "course": { + "id": "course-2" + }, + "name": "Instructor Of Course 2 With Unique Display Name", + "email": "instructorOfCourse2WithUniqueDisplayName@teammates.tmt", + "role": "INSTRUCTOR_PERMISSION_ROLE_COOWNER", + "isDisplayedToStudents": true, + "displayName": "Wilson Kurniawan", + "privileges": { + "courseLevel": { + "canModifyCourse": true, + "canModifyInstructor": true, + "canModifySession": true, + "canModifyStudent": true, + "canViewStudentInSections": true, + "canViewSessionInSections": true, + "canSubmitSessionInSections": true, + "canModifySessionCommentsInSections": true + }, + "sectionLevel": {}, + "sessionLevel": {} + } + }, + "instructor1OfCourse3": { + "id": "00000000-0000-4000-8000-000000000506", + "account": { + "id": "00000000-0000-4000-8000-000000000001" + }, + "course": { + "id": "course-3" + }, + "name": "Instructor 1", + "email": "instr1@teammates.tmt", + "role": "INSTRUCTOR_PERMISSION_ROLE_COOWNER", + "isDisplayedToStudents": true, + "displayName": "Instructor", + "privileges": { + "courseLevel": { + "canModifyCourse": true, + "canModifyInstructor": true, + "canModifySession": true, + "canModifyStudent": true, + "canViewStudentInSections": true, + "canViewSessionInSections": true, + "canSubmitSessionInSections": true, + "canModifySessionCommentsInSections": true + }, + "sectionLevel": {}, + "sessionLevel": {} + } + }, + "unregisteredInstructorOfCourse1": { + "id": "00000000-0000-4000-8000-000000000507", + "course": { + "id": "course-1" + }, + "name": "Unregistered Instructor", + "email": "unregisteredInstructor@teammates.tmt", + "role": "INSTRUCTOR_PERMISSION_ROLE_TUTOR", + "isDisplayedToStudents": true, + "displayName": "Unregistered Instructor", + "privileges": { + "courseLevel": { + "canModifyCourse": false, + "canModifyInstructor": false, + "canModifySession": false, + "canModifyStudent": false, + "canViewStudentInSections": true, + "canViewSessionInSections": true, + "canSubmitSessionInSections": true, + "canModifySessionCommentsInSections": false + }, + "sectionLevel": {}, + "sessionLevel": {} + } + }, + "instructor1OfCourse4": { + "id": "00000000-0000-4000-8000-000000000508", + "account": { + "id": "00000000-0000-4000-8000-000000000001" + }, + "course": { + "id": "course-4" + }, + "name": "Instructor 1", + "email": "instr1@teammates.tmt", + "role": "INSTRUCTOR_PERMISSION_ROLE_COOWNER", + "isDisplayedToStudents": true, + "displayName": "Instructor", + "privileges": { + "courseLevel": { + "canModifyCourse": true, + "canModifyInstructor": true, + "canModifySession": true, + "canModifyStudent": true, + "canViewStudentInSections": true, + "canViewSessionInSections": true, + "canSubmitSessionInSections": true, + "canModifySessionCommentsInSections": true + }, + "sectionLevel": {}, + "sessionLevel": {} + } + }, + "instructor2YetToJoinCourse4": { + "id": "00000000-0000-4000-8000-000000000509", + "course": { + "id": "course-4" + }, + "name": "Instructor 2", + "email": "instr2@teammates.tmt", + "role": "INSTRUCTOR_PERMISSION_ROLE_COOWNER", + "isDisplayedToStudents": true, + "displayName": "Instructor", + "privileges": { + "courseLevel": { + "canModifyCourse": true, + "canModifyInstructor": true, + "canModifySession": true, + "canModifyStudent": true, + "canViewStudentInSections": true, + "canViewSessionInSections": true, + "canSubmitSessionInSections": true, + "canModifySessionCommentsInSections": true + }, + "sectionLevel": {}, + "sessionLevel": {} + } + }, + "instructor3YetToJoinCourse4": { + "id": "00000000-0000-4000-8000-000000000510", + "course": { + "id": "course-4" + }, + "name": "Instructor 3", + "email": "instructor3YetToJoinCourse4@teammates.tmt", + "role": "INSTRUCTOR_PERMISSION_ROLE_COOWNER", + "isDisplayedToStudents": true, + "displayName": "Instructor", + "privileges": { + "courseLevel": { + "canModifyCourse": true, + "canModifyInstructor": true, + "canModifySession": true, + "canModifyStudent": true, + "canViewStudentInSections": true, + "canViewSessionInSections": true, + "canSubmitSessionInSections": true, + "canModifySessionCommentsInSections": true + }, + "sectionLevel": {}, + "sessionLevel": {} + } + } + }, + "students": { + "student1InCourse1": { + "id": "00000000-0000-4000-8000-000000000601", + "account": { + "id": "00000000-0000-4000-8000-000000000101" + }, + "course": { + "id": "course-1" + }, + "team": { + "id": "00000000-0000-4000-8000-000000000301" + }, + "email": "student1@teammates.tmt", + "name": "student1 In Course1", + "comments": "comment for student1Course1" + }, + "student2InCourse1": { + "id": "00000000-0000-4000-8000-000000000602", + "account": { + "id": "00000000-0000-4000-8000-000000000102" + }, + "course": { + "id": "course-1" + }, + "team": { + "id": "00000000-0000-4000-8000-000000000301" + }, + "email": "student2@teammates.tmt", + "name": "student2 In Course1", + "comments": "" + }, + "student3InCourse1": { + "id": "00000000-0000-4000-8000-000000000603", + "account": { + "id": "00000000-0000-4000-8000-000000000103" + }, + "course": { + "id": "course-1" + }, + "team": { + "id": "00000000-0000-4000-8000-000000000301" + }, + "email": "student3@teammates.tmt", + "name": "student3 In Course1", + "comments": "" + }, + "student1InCourse2": { + "id": "00000000-0000-4000-8000-000000000604", + "course": { + "id": "course-2" + }, + "team": { + "id": "00000000-0000-4000-8000-000000000302" + }, + "email": "student1@teammates.tmt", + "name": "student1 In Course2", + "comments": "" + }, + "student1InCourse3": { + "id": "00000000-0000-4000-8000-000000000605", + "email": "student1@teammates.tmt", + "course": { + "id": "course-3" + }, + "team": { + "id": "00000000-0000-4000-8000-000000000304" + }, + "name": "student1 In Course3'\"", + "comments": "comment for student1InCourse3'\"" + }, + "unregisteredStudentInCourse1": { + "id": "00000000-0000-4000-8000-000000000606", + "course": { + "id": "course-1" + }, + "team": { + "id": "00000000-0000-4000-8000-000000000301" + }, + "email": "unregisteredStudentInCourse1@teammates.tmt", + "name": "Unregistered Student In Course1", + "comments": "" + }, + "student1InCourse4": { + "id": "00000000-0000-4000-8000-000000000607", + "account": { + "id": "00000000-0000-4000-8000-000000000101" + }, + "course": { + "id": "course-4" + }, + "team": { + "id": "00000000-0000-4000-8000-000000000301" + }, + "email": "student1@teammates.tmt", + "name": "student1 In Course4", + "comments": "comment for student1Course1" + }, + "student2YetToJoinCourse4": { + "id": "00000000-0000-4000-8000-000000000608", + "course": { + "id": "course-4" + }, + "team": { + "id": "00000000-0000-4000-8000-000000000302" + }, + "email": "student2YetToJoinCourse4@teammates.tmt", + "name": "student2YetToJoinCourse In Course4", + "comments": "" + }, + "student3YetToJoinCourse4": { + "id": "00000000-0000-4000-8000-000000000609", + "course": { + "id": "course-4" + }, + "team": { + "id": "00000000-0000-4000-8000-000000000302" + }, + "email": "student3YetToJoinCourse4@teammates.tmt", + "name": "student3YetToJoinCourse In Course4", + "comments": "" + }, + "studentOfArchivedCourse": { + "id": "00000000-0000-4000-8000-000000000610", + "course": { + "id": "archived-course" + }, + "team": { + "id": "00000000-0000-4000-8000-000000000302" + }, + "email": "studentOfArchivedCourse@teammates.tmt", + "name": "Student In Archived Course", + "comments": "" + } + }, + "feedbackSessions": { + "session1InCourse1": { + "id": "00000000-0000-4000-8000-000000000701", + "course": { + "id": "course-1" + }, + "name": "First feedback session", + "creatorEmail": "instr1@teammates.tmt", + "instructions": "Please please fill in the following questions.", + "startTime": "2012-04-01T22:00:00Z", + "endTime": "2027-04-30T22:00:00Z", + "sessionVisibleFromTime": "2012-03-28T22:00:00Z", + "resultsVisibleFromTime": "2013-05-01T22:00:00Z", + "gracePeriod": 10, + "isOpeningEmailEnabled": true, + "isClosingEmailEnabled": true, + "isPublishedEmailEnabled": true, + "isOpeningSoonEmailSent": true, + "isOpenEmailSent": true, + "isClosingSoonEmailSent": false, + "isClosedEmailSent": false, + "isPublishedEmailSent": true + }, + "session2InTypicalCourse": { + "id": "00000000-0000-4000-8000-000000000702", + "course": { + "id": "course-1" + }, + "name": "Second feedback session", + "creatorEmail": "instr1@teammates.tmt", + "instructions": "Please please fill in the following questions.", + "startTime": "2013-06-01T22:00:00Z", + "endTime": "2026-04-28T22:00:00Z", + "sessionVisibleFromTime": "2013-03-20T22:00:00Z", + "resultsVisibleFromTime": "2026-04-29T22:00:00Z", + "gracePeriod": 5, + "isOpeningEmailEnabled": true, + "isClosingEmailEnabled": true, + "isPublishedEmailEnabled": true, + "isOpeningSoonEmailSent": true, + "isOpenEmailSent": true, + "isClosingSoonEmailSent": false, + "isClosedEmailSent": false, + "isPublishedEmailSent": false + }, + "unpublishedSession1InTypicalCourse": { + "id": "00000000-0000-4000-8000-000000000703", + "course": { + "id": "course-1" + }, + "name": "Unpublished feedback session", + "creatorEmail": "instr1@teammates.tmt", + "instructions": "Please please fill in the following questions.", + "startTime": "2013-06-01T22:00:00Z", + "endTime": "2026-04-28T22:00:00Z", + "sessionVisibleFromTime": "2013-03-20T22:00:00Z", + "resultsVisibleFromTime": "2027-04-27T22:00:00Z", + "gracePeriod": 5, + "isOpeningEmailEnabled": true, + "isClosingEmailEnabled": true, + "isPublishedEmailEnabled": true, + "isOpeningSoonEmailSent": true, + "isOpenEmailSent": true, + "isClosingSoonEmailSent": false, + "isClosedEmailSent": false, + "isPublishedEmailSent": false + }, + "ongoingSession1InCourse1": { + "id": "00000000-0000-4000-8000-000000000704", + "course": { + "id": "course-1" + }, + "name": "Ongoing session 1 in course 1", + "creatorEmail": "instr1@teammates.tmt", + "instructions": "Please please fill in the following questions.", + "startTime": "2012-01-19T22:00:00Z", + "endTime": "2012-01-25T22:00:00Z", + "sessionVisibleFromTime": "2012-01-19T22:00:00Z", + "resultsVisibleFromTime": "2012-02-02T22:00:00Z", + "gracePeriod": 10, + "isOpeningEmailEnabled": true, + "isClosingEmailEnabled": true, + "isPublishedEmailEnabled": true, + "isOpeningSoonEmailSent": true, + "isOpenEmailSent": true, + "isClosingSoonEmailSent": true, + "isClosedEmailSent": true, + "isPublishedEmailSent": true + }, + "ongoingSession2InCourse1": { + "id": "00000000-0000-4000-8000-000000000705", + "course": { + "id": "course-1" + }, + "name": "Ongoing session 2 in course 1", + "creatorEmail": "instr1@teammates.tmt", + "instructions": "Please please fill in the following questions.", + "startTime": "2012-01-26T22:00:00Z", + "endTime": "2012-02-02T22:00:00Z", + "sessionVisibleFromTime": "2012-01-19T22:00:00Z", + "resultsVisibleFromTime": "2012-02-02T22:00:00Z", + "gracePeriod": 10, + "isOpeningEmailEnabled": true, + "isClosingEmailEnabled": true, + "isPublishedEmailEnabled": true, + "isOpeningSoonEmailSent": true, + "isOpenEmailSent": true, + "isClosingSoonEmailSent": true, + "isClosedEmailSent": true, + "isPublishedEmailSent": true + }, + "ongoingSession3InCourse1": { + "id": "00000000-0000-4000-8000-000000000706", + "course": { + "id": "course-1" + }, + "name": "Ongoing session 3 in course 1", + "creatorEmail": "instr1@teammates.tmt", + "instructions": "Please please fill in the following questions.", + "startTime": "2012-01-26T10:00:00Z", + "endTime": "2012-01-27T10:00:00Z", + "sessionVisibleFromTime": "2012-01-19T22:00:00Z", + "resultsVisibleFromTime": "2012-02-02T22:00:00Z", + "gracePeriod": 10, + "isOpeningEmailEnabled": true, + "isClosingEmailEnabled": true, + "isPublishedEmailEnabled": true, + "isOpeningSoonEmailSent": true, + "isOpenEmailSent": true, + "isClosingSoonEmailSent": true, + "isClosedEmailSent": true, + "isPublishedEmailSent": true + }, + "ongoingSession1InCourse3": { + "id": "00000000-0000-4000-8000-000000000707", + "course": { + "id": "course-3" + }, + "name": "Ongoing session 1 in course 3", + "creatorEmail": "instr1@teammates.tmt", + "instructions": "Please please fill in the following questions.", + "startTime": "2012-01-27T22:00:00Z", + "endTime": "2012-02-02T22:00:00Z", + "sessionVisibleFromTime": "2012-01-19T22:00:00Z", + "resultsVisibleFromTime": "2012-02-02T22:00:00Z", + "gracePeriod": 10, + "isOpeningEmailEnabled": true, + "isClosingEmailEnabled": true, + "isPublishedEmailEnabled": true, + "isOpeningSoonEmailSent": true, + "isOpenEmailSent": true, + "isClosingSoonEmailSent": true, + "isClosedEmailSent": true, + "isPublishedEmailSent": true + }, + "ongoingSession2InCourse3": { + "id": "00000000-0000-4000-8000-000000000707", + "course": { + "id": "course-3" + }, + "name": "Ongoing session 2 in course 3", + "creatorEmail": "instr1@teammates.tmt", + "instructions": "Please please fill in the following questions.", + "startTime": "2012-01-19T22:00:00Z", + "endTime": "2027-04-30T22:00:00Z", + "sessionVisibleFromTime": "2012-01-19T22:00:00Z", + "resultsVisibleFromTime": "2012-02-02T22:00:00Z", + "gracePeriod": 10, + "isOpeningEmailEnabled": true, + "isClosingEmailEnabled": true, + "isPublishedEmailEnabled": true, + "isOpeningSoonEmailSent": true, + "isOpenEmailSent": true, + "isClosingSoonEmailSent": true, + "isClosedEmailSent": true, + "isPublishedEmailSent": true + } + }, + "feedbackQuestions": { + "qn1InSession1InCourse1": { + "id": "00000000-0000-4000-8000-000000000801", + "feedbackSession": { + "id": "00000000-0000-4000-8000-000000000701" + }, + "questionDetails": { + "questionType": "TEXT", + "questionText": "What is the best selling point of your product?" + }, + "description": "This is a text question.", + "questionNumber": 1, + "giverType": "STUDENTS", + "recipientType": "SELF", + "numOfEntitiesToGiveFeedbackTo": 1, + "showResponsesTo": ["INSTRUCTORS"], + "showGiverNameTo": ["INSTRUCTORS"], + "showRecipientNameTo": ["INSTRUCTORS"] + }, + "qn2InSession1InCourse1": { + "id": "00000000-0000-4000-8000-000000000802", + "feedbackSession": { + "id": "00000000-0000-4000-8000-000000000701" + }, + "questionDetails": { + "recommendedLength": 0, + "questionType": "TEXT", + "questionText": "Rate 1 other student's product" + }, + "description": "This is a text question.", + "questionNumber": 2, + "giverType": "STUDENTS", + "recipientType": "STUDENTS_EXCLUDING_SELF", + "numOfEntitiesToGiveFeedbackTo": 1, + "showResponsesTo": ["INSTRUCTORS", "RECEIVER"], + "showGiverNameTo": ["INSTRUCTORS"], + "showRecipientNameTo": ["INSTRUCTORS", "RECEIVER"] + }, + "qn3InSession1InCourse1": { + "id": "00000000-0000-4000-8000-000000000803", + "feedbackSession": { + "id": "00000000-0000-4000-8000-000000000701" + }, + "questionDetails": { + "questionType": "TEXT", + "questionText": "My comments on the class" + }, + "description": "This is a text question.", + "questionNumber": 3, + "giverType": "SELF", + "recipientType": "NONE", + "numOfEntitiesToGiveFeedbackTo": -100, + "showResponsesTo": [ + "RECEIVER", + "OWN_TEAM_MEMBERS", + "STUDENTS", + "INSTRUCTORS" + ], + "showGiverNameTo": [ + "RECEIVER", + "OWN_TEAM_MEMBERS", + "STUDENTS", + "INSTRUCTORS" + ], + "showRecipientNameTo": [ + "RECEIVER", + "OWN_TEAM_MEMBERS", + "STUDENTS", + "INSTRUCTORS" + ] + }, + "qn4InSession1InCourse1": { + "id": "00000000-0000-4000-8000-000000000804", + "feedbackSession": { + "id": "00000000-0000-4000-8000-000000000701" + }, + "questionDetails": { + "questionType": "TEXT", + "questionText": "Instructor comments on the class" + }, + "description": "This is a text question.", + "questionNumber": 4, + "giverType": "INSTRUCTORS", + "recipientType": "NONE", + "numOfEntitiesToGiveFeedbackTo": -100, + "showResponsesTo": [ + "RECEIVER", + "OWN_TEAM_MEMBERS", + "STUDENTS", + "INSTRUCTORS" + ], + "showGiverNameTo": [ + "RECEIVER", + "OWN_TEAM_MEMBERS", + "STUDENTS", + "INSTRUCTORS" + ], + "showRecipientNameTo": [ + "RECEIVER", + "OWN_TEAM_MEMBERS", + "STUDENTS", + "INSTRUCTORS" + ] + }, + "qn5InSession1InCourse1": { + "id": "00000000-0000-4000-8000-000000000805", + "feedbackSession": { + "id": "00000000-0000-4000-8000-000000000701" + }, + "questionDetails": { + "recommendedLength": 100, + "questionText": "New format Text question", + "questionType": "TEXT" + }, + "description": "This is a text question.", + "questionNumber": 5, + "giverType": "SELF", + "recipientType": "NONE", + "numOfEntitiesToGiveFeedbackTo": -100, + "showResponsesTo": ["INSTRUCTORS"], + "showGiverNameTo": ["INSTRUCTORS"], + "showRecipientNameTo": ["INSTRUCTORS"] + }, + "qn6InSession1InCourse1NoResponses": { + "id": "00000000-0000-4000-8000-000000000806", + "feedbackSession": { + "id": "00000000-0000-4000-8000-000000000701" + }, + "questionDetails": { + "recommendedLength": 100, + "questionText": "New format Text question", + "questionType": "TEXT" + }, + "description": "Feedback question with no responses", + "questionNumber": 5, + "giverType": "SELF", + "recipientType": "NONE", + "numOfEntitiesToGiveFeedbackTo": -100, + "showResponsesTo": ["INSTRUCTORS"], + "showGiverNameTo": ["INSTRUCTORS"], + "showRecipientNameTo": ["INSTRUCTORS"] + }, + "qn1InSession2InCourse1": { + "id": "00000000-0000-4000-8001-000000000800", + "feedbackSession": { + "id": "00000000-0000-4000-8000-000000000702" + }, + "questionDetails": { + "hasAssignedWeights": false, + "mcqWeights": [], + "mcqOtherWeight": 0.0, + "mcqChoices": ["Great", "Perfect"], + "otherEnabled": false, + "questionDropdownEnabled": false, + "generateOptionsFor": "NONE", + "questionType": "MCQ", + "questionText": "How do you think you did?" + }, + "description": "This is a mcq question.", + "questionNumber": 1, + "giverType": "STUDENTS", + "recipientType": "SELF", + "numOfEntitiesToGiveFeedbackTo": 1, + "showResponsesTo": ["INSTRUCTORS"], + "showGiverNameTo": ["INSTRUCTORS"], + "showRecipientNameTo": ["INSTRUCTORS"] + } + }, + "feedbackResponses": { + "response1ForQ1": { + "id": "00000000-0000-4000-8000-000000000901", + "feedbackQuestion": { + "id": "00000000-0000-4000-8000-000000000801", + "questionDetails": { + "questionType": "TEXT", + "questionText": "What is the best selling point of your product?" + } + }, + "giver": "student1@teammates.tmt", + "recipient": "student1@teammates.tmt", + "giverSection": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "recipientSection": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "answer": { + "questionType": "TEXT", + "answer": "Student 1 self feedback." + } + }, + "response2ForQ1": { + "id": "00000000-0000-4000-8000-000000000902", + "feedbackQuestion": { + "id": "00000000-0000-4000-8000-000000000801", + "questionDetails": { + "questionType": "TEXT", + "questionText": "What is the best selling point of your product?" + } + }, + "giver": "student2@teammates.tmt", + "recipient": "student2@teammates.tmt", + "giverSection": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "recipientSection": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "answer": { + "questionType": "TEXT", + "answer": "Student 2 self feedback." + } + }, + "response1ForQ2": { + "id": "00000000-0000-4000-8000-000000000903", + "feedbackQuestion": { + "id": "00000000-0000-4000-8000-000000000802", + "feedbackSession": { + "id": "00000000-0000-4000-8000-000000000701" + }, + "questionDetails": { + "recommendedLength": 0, + "questionType": "TEXT", + "questionText": "Rate 1 other student's product" + }, + "description": "This is a text question.", + "questionNumber": 2, + "giverType": "STUDENTS", + "recipientType": "STUDENTS_EXCLUDING_SELF", + "numOfEntitiesToGiveFeedbackTo": 1, + "showResponsesTo": ["INSTRUCTORS", "RECEIVER"], + "showGiverNameTo": ["INSTRUCTORS"], + "showRecipientNameTo": ["INSTRUCTORS", "RECEIVER"] + }, + "giver": "student2@teammates.tmt", + "recipient": "student1@teammates.tmt", + "giverSection": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "recipientSection": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "answer": { + "questionType": "TEXT", + "answer": "Student 2's rating of Student 1's project." + } + }, + "response2ForQ2": { + "id": "00000000-0000-4000-8000-000000000904", + "feedbackQuestion": { + "id": "00000000-0000-4000-8000-000000000802", + "feedbackSession": { + "id": "00000000-0000-4000-8000-000000000701" + }, + "questionDetails": { + "recommendedLength": 0, + "questionType": "TEXT", + "questionText": "Rate 1 other student's product" + }, + "description": "This is a text question.", + "questionNumber": 2, + "giverType": "STUDENTS", + "recipientType": "STUDENTS_EXCLUDING_SELF", + "numOfEntitiesToGiveFeedbackTo": 1, + "showResponsesTo": ["INSTRUCTORS", "RECEIVER"], + "showGiverNameTo": ["INSTRUCTORS"], + "showRecipientNameTo": ["INSTRUCTORS", "RECEIVER"] + }, + "giver": "student3@teammates.tmt", + "recipient": "student2@teammates.tmt", + "giverSection": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "recipientSection": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "answer": { + "questionType": "TEXT", + "answer": "Student 3's rating of Student 2's project." + } + }, + "response1ForQ3": { + "id": "00000000-0000-4000-8000-000000000905", + "feedbackQuestion": { + "id": "00000000-0000-4000-8000-000000000803", + "questionDetails": { + "questionType": "TEXT", + "questionText": "My comments on the class" + } + }, + "giver": "student1@teammates.tmt", + "recipient": "student1@teammates.tmt", + "giverSection": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "recipientSection": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "answer": { + "questionType": "TEXT", + "answer": "The class is great." + } + }, + "response1ForQ1InSession2": { + "id": "00000000-0000-4000-8001-000000000901", + "feedbackQuestion": { + "id": "00000000-0000-4000-8001-000000000800", + "questionDetails": { + "hasAssignedWeights": false, + "mcqWeights": [], + "mcqOtherWeight": 0.0, + "mcqChoices": ["Great", "Perfect"], + "otherEnabled": false, + "questionDropdownEnabled": false, + "generateOptionsFor": "NONE", + "questionType": "MCQ", + "questionText": "How do you think you did?" + } + }, + "giver": "student1@teammates.tmt", + "recipient": "student1@teammates.tmt", + "giverSection": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "recipientSection": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "answer": { + "answer": "Great", + "otherFieldContent": "", + "questionType": "MCQ" + } + } + }, + "feedbackResponseComments": { + "comment1ToResponse1ForQ1": { + "feedbackResponse": { + "id": "00000000-0000-4000-8000-000000000901", + "answer": { + "questionType": "TEXT", + "answer": "Student 1 self feedback." + } + }, + "giver": "instr1@teammates.tmt", + "giverType": "INSTRUCTORS", + "giverSection": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "recipientSection": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "commentText": "Instructor 1 comment to student 1 self feedback", + "isVisibilityFollowingFeedbackQuestion": false, + "isCommentFromFeedbackParticipant": false, + "showCommentTo": [], + "showGiverNameTo": [], + "lastEditorEmail": "instr1@teammates.tmt" + }, + "comment2ToResponse1ForQ1": { + "feedbackResponse": { + "id": "00000000-0000-4000-8000-000000000901", + "answer": { + "questionType": "TEXT", + "answer": "Student 1 self feedback." + } + }, + "giver": "student1@teammates.tmt", + "giverType": "STUDENTS", + "giverSection": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "recipientSection": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "commentText": "Student 1 comment to student 1 self feedback", + "isVisibilityFollowingFeedbackQuestion": false, + "isCommentFromFeedbackParticipant": false, + "showCommentTo": [], + "showGiverNameTo": [], + "lastEditorEmail": "student1@teammates.tmt" + }, + "comment2ToResponse2ForQ1": { + "feedbackResponse": { + "id": "00000000-0000-4000-8000-000000000902", + "answer": { + "questionType": "TEXT", + "answer": "Student 2 self feedback." + } + }, + "giver": "instr2@teammates.tmt", + "giverType": "INSTRUCTORS", + "giverSection": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "recipientSection": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "commentText": "Instructor 2 comment to student 2 self feedback", + "isVisibilityFollowingFeedbackQuestion": false, + "isCommentFromFeedbackParticipant": false, + "showCommentTo": [], + "showGiverNameTo": [], + "lastEditorEmail": "instr2@teammates.tmt" + }, + "comment1ToResponse1ForQ2s": { + "feedbackResponse": { + "id": "00000000-0000-4000-8000-000000000903", + "answer": { + "questionType": "TEXT", + "answer": "Student 2 self feedback." + } + }, + "giver": "instr2@teammates.tmt", + "giverType": "INSTRUCTORS", + "giverSection": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "recipientSection": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "commentText": "Instructor 2 comment to student 2 self feedback", + "isVisibilityFollowingFeedbackQuestion": false, + "isCommentFromFeedbackParticipant": false, + "showCommentTo": [], + "showGiverNameTo": [], + "lastEditorEmail": "instr2@teammates.tmt" + }, + "comment1ToResponse1ForQ3": { + "feedbackResponse": { + "id": "00000000-0000-4000-8000-000000000905", + "answer": { + "questionType": "TEXT", + "answer": "The class is great." + } + }, + "giver": "instr1@teammates.tmt", + "giverType": "INSTRUCTORS", + "giverSection": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "recipientSection": { + "id": "00000000-0000-4000-8000-000000000201" + }, + "commentText": "Instructor 1 comment to student 1 self feedback", + "isVisibilityFollowingFeedbackQuestion": false, + "isCommentFromFeedbackParticipant": false, + "showCommentTo": [], + "showGiverNameTo": [], + "lastEditorEmail": "instr1@teammates.tmt" + } + }, + "notifications": { + "notification1": { + "id": "00000000-0000-4000-8000-000000001101", + "startTime": "2011-01-01T00:00:00Z", + "endTime": "2099-01-01T00:00:00Z", + "style": "DANGER", + "targetUser": "GENERAL", + "title": "A deprecation note", + "message": "

Deprecation happens in three minutes

", + "shown": false + } + }, + "readNotifications": { + "notification1Instructor1": { + "id": "00000000-0000-4000-8000-000000001201", + "account": { + "id": "00000000-0000-4000-8000-000000000001" + }, + "notification": { + "id": "00000000-0000-4000-8000-000000001101" + } + }, + "notification1Student1": { + "id": "00000000-0000-4000-8000-000000001101", + "account": { + "id": "00000000-0000-4000-8000-000000000002" + }, + "notification": { + "id": "00000000-0000-4000-8000-000000001101" + } + } + } +} From ab2025927e9750a60e5bc0eb7ef1cf6187057bf5 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Tue, 20 Feb 2024 18:17:46 +0800 Subject: [PATCH 17/42] Add IT for GetSessionResultsAction --- .../ui/webapi/GetSessionResultsActionIT.java | 366 ++++++++++++++++++ 1 file changed, 366 insertions(+) create mode 100644 src/it/java/teammates/it/ui/webapi/GetSessionResultsActionIT.java diff --git a/src/it/java/teammates/it/ui/webapi/GetSessionResultsActionIT.java b/src/it/java/teammates/it/ui/webapi/GetSessionResultsActionIT.java new file mode 100644 index 00000000000..d27e094dee4 --- /dev/null +++ b/src/it/java/teammates/it/ui/webapi/GetSessionResultsActionIT.java @@ -0,0 +1,366 @@ +package teammates.it.ui.webapi; + +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import teammates.common.datatransfer.FeedbackResultFetchType; +import teammates.common.util.Const; +import teammates.common.util.HibernateUtil; +import teammates.common.util.JsonUtils; +import teammates.storage.sqlentity.Course; +import teammates.storage.sqlentity.FeedbackSession; +import teammates.storage.sqlentity.Instructor; +import teammates.storage.sqlentity.Section; +import teammates.storage.sqlentity.Student; +import teammates.ui.output.SessionResultsData; +import teammates.ui.request.Intent; +import teammates.ui.webapi.GetSessionResultsAction; +import teammates.ui.webapi.JsonResult; + +/** + * SUT: {@link GetSessionResultsAction}. + */ +public class GetSessionResultsActionIT extends BaseActionIT { + + @Override + protected String getActionUri() { + return Const.ResourceURIs.RESULT; + } + + @Override + protected String getRequestMethod() { + return GET; + } + + @Override + @BeforeMethod + protected void setUp() throws Exception { + super.setUp(); + logoutUser(); + persistDataBundle(typicalBundle); + HibernateUtil.flushSession(); + } + + @Override + @Test + protected void testExecute() { + Instructor instructor = typicalBundle.instructors.get("instructor1OfCourse1"); + loginAsInstructor(instructor.getGoogleId()); + + ______TS("Typical: Instructor accesses results of their course"); + + FeedbackSession accessibleFeedbackSession = typicalBundle.feedbackSessions.get("session1InCourse1"); + String[] submissionParams = new String[] { + Const.ParamsNames.FEEDBACK_SESSION_NAME, accessibleFeedbackSession.getName(), + Const.ParamsNames.COURSE_ID, accessibleFeedbackSession.getCourse().getId(), + Const.ParamsNames.INTENT, Intent.FULL_DETAIL.name(), + }; + + GetSessionResultsAction a = getAction(submissionParams); + JsonResult r = getJsonResult(a); + + SessionResultsData output = (SessionResultsData) r.getOutput(); + + SessionResultsData expectedResults = SessionResultsData.initForInstructor( + logic.getSessionResultsForCourse(accessibleFeedbackSession, + accessibleFeedbackSession.getCourse().getId(), + instructor.getEmail(), + null, null, FeedbackResultFetchType.BOTH)); + + assertTrue(isSessionResultsDataEqual(expectedResults, output)); + + ______TS("Typical: Instructor accesses results of their course with breakdown"); + + Set
sections = new HashSet<>(); + typicalBundle.feedbackResponses.values().forEach(resp -> { + sections.add(resp.getGiverSection()); + sections.add(resp.getRecipientSection()); + }); + + for (FeedbackResultFetchType fetchType : FeedbackResultFetchType.values()) { + for (Section section : sections) { + submissionParams = new String[] { + Const.ParamsNames.FEEDBACK_SESSION_NAME, accessibleFeedbackSession.getName(), + Const.ParamsNames.COURSE_ID, accessibleFeedbackSession.getCourse().getId(), + Const.ParamsNames.INTENT, Intent.FULL_DETAIL.name(), + Const.ParamsNames.FEEDBACK_RESULTS_GROUPBYSECTION, section.getName(), + Const.ParamsNames.FEEDBACK_RESULTS_SECTION_BY_GIVER_RECEIVER, fetchType.name(), + }; + + a = getAction(submissionParams); + r = getJsonResult(a); + + output = (SessionResultsData) r.getOutput(); + + expectedResults = SessionResultsData.initForInstructor( + logic.getSessionResultsForCourse(accessibleFeedbackSession, + accessibleFeedbackSession.getCourse().getId(), + instructor.getEmail(), + null, section.getName(), fetchType)); + + assertTrue(isSessionResultsDataEqual(expectedResults, output)); + } + } + + ______TS("Typical: Student accesses results of their course"); + + Student student = typicalBundle.students.get("student1InCourse1"); + loginAsStudent(student.getGoogleId()); + + submissionParams = new String[] { + Const.ParamsNames.FEEDBACK_SESSION_NAME, accessibleFeedbackSession.getName(), + Const.ParamsNames.COURSE_ID, accessibleFeedbackSession.getCourse().getId(), + Const.ParamsNames.INTENT, Intent.STUDENT_RESULT.name(), + }; + + a = getAction(submissionParams); + r = getJsonResult(a); + + output = (SessionResultsData) r.getOutput(); + expectedResults = SessionResultsData.initForStudent( + logic.getSessionResultsForUser(accessibleFeedbackSession, + accessibleFeedbackSession.getCourse().getId(), + student.getEmail(), + false, null), + student); + + assertTrue(isSessionResultsDataEqual(expectedResults, output)); + } + + @Override + protected void testAccessControl() { + // See test cases below + } + + @Test + protected void testAccessControl_invalidIntent_failure() { + String[] submissionParams; + FeedbackSession publishedFeedbackSession = typicalBundle.feedbackSessions.get("session1InCourse1"); + submissionParams = new String[] { + Const.ParamsNames.FEEDBACK_SESSION_NAME, publishedFeedbackSession.getName(), + Const.ParamsNames.COURSE_ID, publishedFeedbackSession.getCourse().getId(), + Const.ParamsNames.INTENT, Intent.INSTRUCTOR_SUBMISSION.name(), + }; + verifyHttpParameterFailure(submissionParams); + submissionParams = new String[] { + Const.ParamsNames.FEEDBACK_SESSION_NAME, publishedFeedbackSession.getName(), + Const.ParamsNames.COURSE_ID, publishedFeedbackSession.getCourse().getId(), + Const.ParamsNames.INTENT, Intent.STUDENT_SUBMISSION.name(), + }; + verifyHttpParameterFailure(submissionParams); + } + + @Test + protected void testAccessControl_resultsNotPublished_shouldNotBeAccessible() { + String[] submissionParams; + FeedbackSession inaccessibleFeedbackSession = typicalBundle.feedbackSessions.get( + "unpublishedSession1InTypicalCourse"); + + ______TS("Inaccessible for authenticated instructor when unpublished"); + submissionParams = new String[] { + Const.ParamsNames.FEEDBACK_SESSION_NAME, inaccessibleFeedbackSession.getName(), + Const.ParamsNames.COURSE_ID, inaccessibleFeedbackSession.getCourse().getId(), + Const.ParamsNames.INTENT, Intent.INSTRUCTOR_RESULT.name(), + }; + verifyCannotAccess(submissionParams); + + ______TS("Inaccessible for authenticated student when unpublished"); + submissionParams = new String[] { + Const.ParamsNames.FEEDBACK_SESSION_NAME, inaccessibleFeedbackSession.getName(), + Const.ParamsNames.COURSE_ID, inaccessibleFeedbackSession.getCourse().getId(), + Const.ParamsNames.INTENT, Intent.STUDENT_RESULT.name(), + }; + Student student1InCourse1 = typicalBundle.students.get("student1InCourse1"); + loginAsStudent(student1InCourse1.getGoogleId()); + verifyCannotAccess(submissionParams); + } + + @Test + protected void testAccessControl_resultsPublished_shouldBeAccessible() throws Exception { + String[] submissionParams; + FeedbackSession publishedFeedbackSession = typicalBundle.feedbackSessions.get("session1InCourse1"); + Course course = publishedFeedbackSession.getCourse(); + + ______TS("Accessible for authenticated instructor when published"); + submissionParams = new String[] { + Const.ParamsNames.FEEDBACK_SESSION_NAME, publishedFeedbackSession.getName(), + Const.ParamsNames.COURSE_ID, course.getId(), + Const.ParamsNames.INTENT, Intent.INSTRUCTOR_RESULT.name(), + }; + verifyAccessibleForInstructorsOfTheSameCourse(course, submissionParams); + verifyInaccessibleForInstructorsOfOtherCourses(course, submissionParams); + + ______TS("Accessible for authenticated student when published"); + submissionParams = new String[] { + Const.ParamsNames.FEEDBACK_SESSION_NAME, publishedFeedbackSession.getName(), + Const.ParamsNames.COURSE_ID, course.getId(), + Const.ParamsNames.INTENT, Intent.STUDENT_RESULT.name(), + }; + verifyAccessibleForStudentsOfTheSameCourse(course, submissionParams); + verifyInaccessibleForStudentsOfOtherCourse(course, submissionParams); + } + + private boolean isSessionResultsDataEqual(SessionResultsData self, SessionResultsData other) { + List thisQuestions = self.getQuestions(); + List otherQuestions = other.getQuestions(); + if (thisQuestions.size() != otherQuestions.size()) { + return false; + } + for (int i = 0; i < thisQuestions.size(); i++) { + SessionResultsData.QuestionOutput thisQuestion = thisQuestions.get(i); + SessionResultsData.QuestionOutput otherQuestion = otherQuestions.get(i); + if (!isQuestionOutputEqual(thisQuestion, otherQuestion)) { + return false; + } + } + return true; + } + + private boolean isQuestionOutputEqual(SessionResultsData.QuestionOutput self, + SessionResultsData.QuestionOutput other) { + if (!JsonUtils.toJson(self.getFeedbackQuestion()).equals(JsonUtils.toJson(self.getFeedbackQuestion())) + || !self.getQuestionStatistics().equals(other.getQuestionStatistics())) { + return false; + } + List thisResponses; + List otherResponses; + thisResponses = self.getAllResponses(); + otherResponses = other.getAllResponses(); + if (thisResponses.size() != otherResponses.size()) { + return false; + } + for (int j = 0; j < thisResponses.size(); j++) { + if (!isResponseOutputEqual(thisResponses.get(j), otherResponses.get(j))) { + return false; + } + } + return true; + } + + private boolean isResponseOutputEqual(SessionResultsData.ResponseOutput self, + SessionResultsData.ResponseOutput other) { + return self.getGiver().equals(other.getGiver()) + && self.getGiverTeam().equals(other.getGiverTeam()) + && self.getGiverSection().equals(other.getGiverSection()) + && self.getRecipient().equals(other.getRecipient()) + && self.getRecipientTeam().equals(other.getRecipientTeam()) + && self.getRecipientSection().equals(other.getRecipientSection()) + && self.getResponseDetails().getJsonString().equals(other.getResponseDetails().getJsonString()); + } + + @Test + public void testAccessControl_withRegistrationKey_shouldPass() throws Exception { + Course typicalCourse1 = typicalBundle.courses.get("course1"); + FeedbackSession feedbackSession = typicalBundle.feedbackSessions.get("session1InCourse1"); + Student student1 = typicalBundle.students.get("student1InCourse1"); + student1 = logic.getStudentForEmail(student1.getCourse().getId(), student1.getEmail()); + + String[] submissionParams = new String[] { + Const.ParamsNames.COURSE_ID, typicalCourse1.getId(), + Const.ParamsNames.FEEDBACK_SESSION_NAME, feedbackSession.getName(), + Const.ParamsNames.INTENT, Intent.STUDENT_RESULT.toString(), + Const.ParamsNames.REGKEY, student1.getRegKey(), + }; + + verifyAccessibleForUnregisteredUsers(submissionParams); + } + + @Test + public void testAccessControl_withoutCorrectAuthInfoAccessStudentResult_shouldFail() throws Exception { + Course typicalCourse1 = typicalBundle.courses.get("course1"); + FeedbackSession feedbackSession = typicalBundle.feedbackSessions.get("session1InCourse1"); + + String[] submissionParams = new String[] { + Const.ParamsNames.COURSE_ID, typicalCourse1.getId(), + Const.ParamsNames.FEEDBACK_SESSION_NAME, feedbackSession.getName(), + Const.ParamsNames.INTENT, Intent.STUDENT_RESULT.toString(), + }; + + verifyInaccessibleForUnregisteredUsers(submissionParams); + } + + @Test + public void testAccessControl_studentAccessOwnCourseSessionResult_shouldPass() throws Exception { + Student student1InCourse1 = typicalBundle.students.get("student1InCourse1"); + Course typicalCourse1 = typicalBundle.courses.get("course1"); + FeedbackSession feedbackSession = typicalBundle.feedbackSessions.get("session1InCourse1"); + + String[] submissionParams = new String[] { + Const.ParamsNames.COURSE_ID, typicalCourse1.getId(), + Const.ParamsNames.FEEDBACK_SESSION_NAME, feedbackSession.getName(), + Const.ParamsNames.INTENT, Intent.STUDENT_RESULT.toString(), + }; + + loginAsStudent(student1InCourse1.getGoogleId()); + verifyCanAccess(submissionParams); + } + + @Test + public void testAccessControl_studentAccessUnpublishedSessionStudentResult_shouldFail() { + Student student1InCourse1 = typicalBundle.students.get("student1InCourse1"); + Course typicalCourse = typicalBundle.courses.get("course1"); + FeedbackSession feedbackSession = typicalBundle.feedbackSessions.get("session2InTypicalCourse"); + + String[] submissionParams = new String[] { + Const.ParamsNames.COURSE_ID, typicalCourse.getId(), + Const.ParamsNames.FEEDBACK_SESSION_NAME, feedbackSession.getName(), + Const.ParamsNames.INTENT, Intent.STUDENT_RESULT.toString(), + }; + loginAsStudent(student1InCourse1.getGoogleId()); + verifyCannotAccess(submissionParams); + } + + @Test + public void testAccessControl_accessStudentSessionResultWithMasqueradeMode_shouldPass() throws Exception { + Student student1InCourse1 = typicalBundle.students.get("student1InCourse1"); + Course typicalCourse1 = typicalBundle.courses.get("course1"); + FeedbackSession feedbackSession = typicalBundle.feedbackSessions.get("session1InCourse1"); + String[] submissionParams = new String[] { + Const.ParamsNames.COURSE_ID, typicalCourse1.getId(), + Const.ParamsNames.FEEDBACK_SESSION_NAME, feedbackSession.getName(), + Const.ParamsNames.INTENT, Intent.STUDENT_RESULT.toString(), + }; + + loginAsAdmin(); + verifyCanMasquerade(student1InCourse1.getGoogleId(), submissionParams); + } + + @Test + public void testAccessControl_studentAccessOtherCourseSessionResult_shouldFail() { + Student studentInOtherCourse = typicalBundle.students.get("student2InCourse1"); + Course otherCourse = typicalBundle.courses.get("course1"); + Course course = typicalBundle.courses.get("course3"); + FeedbackSession feedbackSession = typicalBundle.feedbackSessions.get("ongoingSession1InCourse3"); + + String[] submissionParams = new String[] { + Const.ParamsNames.COURSE_ID, course.getId(), + Const.ParamsNames.FEEDBACK_SESSION_NAME, feedbackSession.getName(), + Const.ParamsNames.INTENT, Intent.STUDENT_RESULT.toString(), + }; + + loginAsStudent(studentInOtherCourse.getGoogleId()); + verifyCannotAccess(submissionParams); + + // Malicious api call using course Id of the student to bypass the check + submissionParams[1] = otherCourse.getId(); + verifyEntityNotFoundAcl(submissionParams); + } + + @Test + public void testAccessControl_instructorAccessHisCourseFullDetail_shouldPass() throws Exception { + Course typicalCourse1 = typicalBundle.courses.get("course1"); + FeedbackSession feedbackSession = typicalBundle.feedbackSessions.get("session1InCourse1"); + + String[] submissionParams = new String[] { + Const.ParamsNames.COURSE_ID, typicalCourse1.getId(), + Const.ParamsNames.FEEDBACK_SESSION_NAME, feedbackSession.getName(), + Const.ParamsNames.INTENT, Intent.FULL_DETAIL.toString(), + }; + verifyOnlyInstructorsOfTheSameCourseCanAccess(typicalCourse1, submissionParams); + } + +} From 06c6b610f7b213df383a9e15b6dd07c20a1dc630 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Tue, 20 Feb 2024 18:17:58 +0800 Subject: [PATCH 18/42] Fix action logic --- .../java/teammates/sqllogic/api/Logic.java | 6 +-- .../core/FeedbackResponseCommentsLogic.java | 12 +++--- .../sqllogic/core/FeedbackResponsesLogic.java | 40 +++++++++---------- .../sqlapi/FeedbackResponseCommentsDb.java | 18 ++++----- .../storage/sqlapi/FeedbackResponsesDb.java | 36 ++++++++++------- .../ui/webapi/GetSessionResultsAction.java | 9 +++-- 6 files changed, 64 insertions(+), 57 deletions(-) diff --git a/src/main/java/teammates/sqllogic/api/Logic.java b/src/main/java/teammates/sqllogic/api/Logic.java index 96dc42cd51d..2fd6fabec36 100644 --- a/src/main/java/teammates/sqllogic/api/Logic.java +++ b/src/main/java/teammates/sqllogic/api/Logic.java @@ -1196,17 +1196,17 @@ public List getFeedbackQuestionsForInstructors( * Gets the session result for a feedback session. * * @see FeedbackResponsesLogic#getSessionResultsForCourse( - * FeedbackSession, String, String, String, String, FeedbackResultFetchType) + * FeedbackSession, String, String, String, Section, FeedbackResultFetchType) */ public SqlSessionResultsBundle getSessionResultsForCourse( FeedbackSession feedbackSession, String courseId, String userEmail, - @Nullable UUID questionId, @Nullable String section, @Nullable FeedbackResultFetchType fetchType) { + @Nullable UUID questionId, @Nullable String sectionName, @Nullable FeedbackResultFetchType fetchType) { assert feedbackSession != null; assert courseId != null; assert userEmail != null; return feedbackResponsesLogic.getSessionResultsForCourse( - feedbackSession, courseId, userEmail, questionId, section, fetchType); + feedbackSession, courseId, userEmail, questionId, sectionName, fetchType); } /** diff --git a/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java b/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java index b9b2bfced24..dc580779f50 100644 --- a/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java +++ b/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java @@ -157,11 +157,11 @@ public void updateFeedbackResponseCommentsForResponse(FeedbackResponse response) * @return a list of feedback response comments */ public List getFeedbackResponseCommentForSessionInSection( - String courseId, String feedbackSessionName, @Nullable String section) { - if (section == null) { + String courseId, String feedbackSessionName, @Nullable String sectionName) { + if (sectionName == null) { return frcDb.getFeedbackResponseCommentsForSession(courseId, feedbackSessionName); } - return frcDb.getFeedbackResponseCommentsForSessionInSection(courseId, feedbackSessionName, section); + return frcDb.getFeedbackResponseCommentsForSessionInSection(courseId, feedbackSessionName, sectionName); } /** @@ -172,11 +172,11 @@ public List getFeedbackResponseCommentForSessionInSecti * @return a list of feedback response comments */ public List getFeedbackResponseCommentForQuestionInSection( - UUID questionId, @Nullable String section) { - if (section == null) { + UUID questionId, @Nullable String sectionName) { + if (sectionName == null) { return frcDb.getFeedbackResponseCommentsForQuestion(questionId); } - return frcDb.getFeedbackResponseCommentsForQuestionInSection(questionId, section); + return frcDb.getFeedbackResponseCommentsForQuestionInSection(questionId, sectionName); } /** diff --git a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java index d400fd5d31a..f2e525732ac 100644 --- a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java +++ b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java @@ -523,7 +523,7 @@ private List getQuestionsForSession( } private SqlSessionResultsBundle buildResultsBundle( - boolean isCourseWide, FeedbackSession feedbackSession, String courseId, String section, UUID questionId, + boolean isCourseWide, FeedbackSession feedbackSession, String courseId, String sectionName, UUID questionId, boolean isInstructor, String userEmail, Instructor instructor, Student student, SqlCourseRoster roster, List allQuestions, List allResponses) { @@ -532,9 +532,9 @@ private SqlSessionResultsBundle buildResultsBundle( List allComments; if (questionId == null) { allComments = frcLogic.getFeedbackResponseCommentForSessionInSection( - courseId, feedbackSession.getName(), section); + courseId, feedbackSession.getName(), sectionName); } else { - allComments = frcLogic.getFeedbackResponseCommentForQuestionInSection(questionId, section); + allComments = frcLogic.getFeedbackResponseCommentForQuestionInSection(questionId, sectionName); } RequestTracer.checkRemainingTime(); @@ -614,7 +614,7 @@ private SqlSessionResultsBundle buildResultsBundle( if (isCourseWide) { missingResponses = buildMissingResponses( courseId, feedbackSession, instructor, responseGiverVisibilityTable, - responseRecipientVisibilityTable, relatedQuestions, existingResponses, roster, section); + responseRecipientVisibilityTable, relatedQuestions, existingResponses, roster, sectionName); } RequestTracer.checkRemainingTime(); @@ -630,13 +630,13 @@ private SqlSessionResultsBundle buildResultsBundle( * @param courseId the ID of the course * @param instructorEmail the instructor viewing the feedback session * @param questionId if not null, will only return partial bundle for the question - * @param section if not null, will only return partial bundle for the section + * @param sectionName if not null, will only return partial bundle for the section * @param fetchType if not null, will fetch responses by giver, receiver sections, or both * @return the session result bundle */ public SqlSessionResultsBundle getSessionResultsForCourse( FeedbackSession feedbackSession, String courseId, String instructorEmail, - @Nullable UUID questionId, @Nullable String section, @Nullable FeedbackResultFetchType fetchType) { + @Nullable UUID questionId, @Nullable String sectionName, @Nullable FeedbackResultFetchType fetchType) { SqlCourseRoster roster = new SqlCourseRoster( usersLogic.getStudentsForCourse(courseId), @@ -650,16 +650,16 @@ public SqlSessionResultsBundle getSessionResultsForCourse( List allResponses; // load all response for instructors and passively filter them later if (questionId == null) { - allResponses = getFeedbackResponsesForSessionInSection(feedbackSession, courseId, section, fetchType); + allResponses = getFeedbackResponsesForSessionInSection(feedbackSession, courseId, sectionName, fetchType); } else { - allResponses = getFeedbackResponsesForQuestionInSection(questionId, section, fetchType); + allResponses = getFeedbackResponsesForQuestionInSection(questionId, sectionName, fetchType); } RequestTracer.checkRemainingTime(); // consider the current viewing user Instructor instructor = usersLogic.getInstructorForEmail(courseId, instructorEmail); - return buildResultsBundle(true, feedbackSession, courseId, section, questionId, true, instructorEmail, + return buildResultsBundle(true, feedbackSession, courseId, sectionName, questionId, true, instructorEmail, instructor, null, roster, allQuestions, allResponses); } @@ -721,7 +721,7 @@ private List buildMissingResponses( Map responseGiverVisibilityTable, Map responseRecipientVisibilityTable, List relatedQuestions, - List existingResponses, SqlCourseRoster courseRoster, @Nullable String section) { + List existingResponses, SqlCourseRoster courseRoster, @Nullable String sectionName) { // first get all possible giver recipient pairs Map>> questionCompleteGiverRecipientMap = new HashMap<>(); @@ -761,9 +761,9 @@ private List buildMissingResponses( SqlCourseRoster.ParticipantInfo recipientInfo = courseRoster.getInfoForIdentifier(recipientIdentifier); // skip responses not in current section - if (section != null - && !giverInfo.getSectionName().equals(section) - && !recipientInfo.getSectionName().equals(section)) { + if (sectionName != null + && !giverInfo.getSectionName().equals(sectionName) + && !recipientInfo.getSectionName().equals(sectionName)) { continue; } @@ -954,17 +954,17 @@ List getFeedbackResponsesForSession( * * @param feedbackSession the session * @param courseId the course ID of the session - * @param section if null, will retrieve all responses in the session + * @param sectionName if null, will retrieve all responses in the session * @param fetchType if not null, will retrieve responses by giver, receiver sections, or both * @return a list of responses */ public List getFeedbackResponsesForSessionInSection( - FeedbackSession feedbackSession, String courseId, @Nullable String section, + FeedbackSession feedbackSession, String courseId, @Nullable String sectionName, @Nullable FeedbackResultFetchType fetchType) { - if (section == null) { + if (sectionName == null) { return getFeedbackResponsesForSession(feedbackSession, courseId); } - return frDb.getFeedbackResponsesForSessionInSection(feedbackSession, courseId, section, fetchType); + return frDb.getFeedbackResponsesForSessionInSection(feedbackSession, courseId, sectionName, fetchType); } /** @@ -982,11 +982,11 @@ public List getFeedbackResponsesForQuestion(UUID feedbackQuest * @return a list of responses */ public List getFeedbackResponsesForQuestionInSection( - UUID feedbackQuestionId, @Nullable String section, FeedbackResultFetchType fetchType) { - if (section == null) { + UUID feedbackQuestionId, @Nullable String sectionName, FeedbackResultFetchType fetchType) { + if (sectionName == null) { return getFeedbackResponsesForQuestion(feedbackQuestionId); } - return frDb.getFeedbackResponsesForQuestionInSection(feedbackQuestionId, section, fetchType); + return frDb.getFeedbackResponsesForQuestionInSection(feedbackQuestionId, sectionName, fetchType); } /** diff --git a/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java b/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java index 4eeeb694458..e4e15194675 100644 --- a/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java +++ b/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java @@ -18,6 +18,7 @@ import jakarta.persistence.criteria.CriteriaBuilder; import jakarta.persistence.criteria.CriteriaQuery; import jakarta.persistence.criteria.Join; +import jakarta.persistence.criteria.ListJoin; import jakarta.persistence.criteria.Root; import jakarta.persistence.criteria.Subquery; @@ -212,10 +213,11 @@ public List getFeedbackResponseCommentsForSession( Join frJoin = root.join("feedbackResponse"); Join fqJoin = frJoin.join("feedbackQuestion"); Join fsJoin = fqJoin.join("feedbackSession"); + Join cJoin = fsJoin.join("course"); cq.select(root) .where(cb.and( - cb.equal(fsJoin.get("courseId"), courseId), + cb.equal(cJoin.get("id"), courseId), cb.equal(fsJoin.get("name"), feedbackSessionName) )); @@ -257,16 +259,13 @@ public List getFeedbackResponseCommentsForSessionInSect Join fqJoin = frJoin.join("feedbackQuestion"); Join fsJoin = fqJoin.join("feedbackSession"); Join cJoin = fsJoin.join("course"); - - Subquery
sq = cq.subquery(Section.class); - Root
sqRoot = sq.from(Section.class); - sq.select(sqRoot).where(cb.equal(sqRoot.get("name"), sectionName)); + ListJoin sectionsJoin = cJoin.joinList("sections"); cq.select(root) .where(cb.and( cb.equal(cJoin.get("id"), courseId), cb.equal(fsJoin.get("name"), feedbackSessionName), - cb.in(cJoin.get("sections")).value(sq) + cb.in(cb.literal(sectionName)).value(sectionsJoin.get("name")) )); return HibernateUtil.createQuery(cq).getResultList(); @@ -286,15 +285,12 @@ public List getFeedbackResponseCommentsForQuestionInSec Join fqJoin = root.join("feedbackQuestion"); Join fsJoin = fqJoin.join("feedbackSession"); Join cJoin = fsJoin.join("course"); - - Subquery
sq = cq.subquery(Section.class); - Root
sqRoot = sq.from(Section.class); - sq.select(sqRoot).where(cb.equal(sqRoot.get("name"), sectionName)); + ListJoin sectionsJoin = cJoin.joinList("sections"); cq.select(root) .where(cb.and( cb.equal(fqJoin.get("id"), questionId), - cb.in(cJoin.get("sections")).value(sq) + cb.in(cb.literal(sectionName)).value(sectionsJoin.get("name")) )); return HibernateUtil.createQuery(cq).getResultList(); diff --git a/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java b/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java index c73dcce3c55..f649865a083 100644 --- a/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java +++ b/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java @@ -15,7 +15,7 @@ import teammates.storage.sqlentity.FeedbackQuestion; import teammates.storage.sqlentity.FeedbackResponse; import teammates.storage.sqlentity.FeedbackSession; - +import teammates.storage.sqlentity.Section; import jakarta.persistence.criteria.CriteriaBuilder; import jakarta.persistence.criteria.CriteriaDelete; import jakarta.persistence.criteria.CriteriaQuery; @@ -236,10 +236,11 @@ public List getFeedbackResponsesForRecipientForQuestion( CriteriaBuilder cb = HibernateUtil.getCriteriaBuilder(); CriteriaQuery cq = cb.createQuery(FeedbackResponse.class); Root root = cq.from(FeedbackResponse.class); + Join fqJoin = root.join("feedbackQuestion"); cq.select(root) .where(cb.and( - cb.equal(root.get("questionId"), questionId), + cb.equal(fqJoin.get("id"), questionId), cb.equal(root.get("recipient"), recipient) )); @@ -255,10 +256,11 @@ public List getFeedbackResponsesForQuestion(UUID questionId) { CriteriaBuilder cb = HibernateUtil.getCriteriaBuilder(); CriteriaQuery cq = cb.createQuery(FeedbackResponse.class); Root root = cq.from(FeedbackResponse.class); + Join fqJoin = root.join("feedbackQuestion"); cq.select(root) .where(cb.and( - cb.equal(root.get("questionId"), questionId) + cb.equal(fqJoin.get("id"), questionId) )); return HibernateUtil.createQuery(cq).getResultList(); @@ -269,10 +271,10 @@ public List getFeedbackResponsesForQuestion(UUID questionId) { * Optionally, retrieves by either giver, receiver sections, or both. */ public List getFeedbackResponsesForSessionInSection( - FeedbackSession feedbackSession, String courseId, String sectionId, FeedbackResultFetchType fetchType) { + FeedbackSession feedbackSession, String courseId, String sectionName, FeedbackResultFetchType fetchType) { assert feedbackSession != null; assert courseId != null; - assert sectionId != null; + assert sectionName != null; assert fetchType != null; CriteriaBuilder cb = HibernateUtil.getCriteriaBuilder(); @@ -280,22 +282,25 @@ public List getFeedbackResponsesForSessionInSection( Root root = cq.from(FeedbackResponse.class); Join fqJoin = root.join("feedbackQuestion"); Join fsJoin = fqJoin.join("feedbackSession"); + Join cJoin = fsJoin.join("course"); // unless specified by fetchType, do not filter by giver/recipient section Predicate giverSectionFilter = cb.isTrue(cb.literal(true)); Predicate recipientSectionFilter = cb.isTrue(cb.literal(true)); + Join giverJoin = root.join("giverSection"); + Join recipientJoin = root.join("recipientSection"); if (fetchType.shouldFetchByGiver()) { - giverSectionFilter = cb.equal(root.get("giverSectionId"), sectionId); + giverSectionFilter = cb.equal(giverJoin.get("name"), sectionName); } if (fetchType.shouldFetchByReceiver()) { - giverSectionFilter = cb.equal(root.get("recipientSectionId"), sectionId); + giverSectionFilter = cb.equal(recipientJoin.get("name"), sectionName); } cq.select(root) .where(cb.and( - cb.equal(fqJoin.get("sessionId"), feedbackSession.getId()), - cb.equal(fsJoin.get("courseId"), courseId), + cb.equal(fsJoin.get("id"), feedbackSession.getId()), + cb.equal(cJoin.get("id"), courseId), giverSectionFilter, recipientSectionFilter )); @@ -307,9 +312,9 @@ public List getFeedbackResponsesForSessionInSection( * Gets all feedback responses of a question in a specific section. */ public List getFeedbackResponsesForQuestionInSection( - UUID questionId, String sectionId, FeedbackResultFetchType fetchType) { + UUID questionId, String sectionName, FeedbackResultFetchType fetchType) { assert questionId != null; - assert sectionId != null; + assert sectionName != null; assert fetchType != null; CriteriaBuilder cb = HibernateUtil.getCriteriaBuilder(); @@ -320,12 +325,14 @@ public List getFeedbackResponsesForQuestionInSection( // unless specified by fetchType, do not filter by giver/recipient section Predicate giverSectionFilter = cb.isTrue(cb.literal(true)); Predicate recipientSectionFilter = cb.isTrue(cb.literal(true)); + Join giverJoin = root.join("giverSection"); + Join recipientJoin = root.join("recipientSection"); if (fetchType.shouldFetchByGiver()) { - giverSectionFilter = cb.equal(root.get("giverSectionId"), sectionId); + giverSectionFilter = cb.equal(giverJoin.get("name"), sectionName); } if (fetchType.shouldFetchByReceiver()) { - giverSectionFilter = cb.equal(root.get("recipientSectionId"), sectionId); + giverSectionFilter = cb.equal(recipientJoin.get("name"), sectionName); } cq.select(root) @@ -351,11 +358,12 @@ public List getFeedbackResponsesForSession( Root root = cq.from(FeedbackResponse.class); Join fqJoin = root.join("feedbackQuestion"); Join fsJoin = fqJoin.join("feedbackSession"); + Join cJoin = fsJoin.join("course"); cq.select(root) .where(cb.and( cb.equal(fsJoin.get("id"), feedbackSession.getId()), - cb.equal(fsJoin.get("courseId"), courseId) + cb.equal(cJoin.get("id"), courseId) )); return HibernateUtil.createQuery(cq).getResultList(); diff --git a/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java b/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java index 70f780caa27..785c911b32e 100644 --- a/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java +++ b/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java @@ -116,7 +116,11 @@ public JsonResult execute() { Intent intent = Intent.valueOf(getNonNullRequestParamValue(Const.ParamsNames.INTENT)); if (isCourseMigrated(courseId)) { - return executeWithSql(courseId, feedbackSessionName, questionId, selectedSection, fetchType, intent); + UUID questionUuid = null; + if (questionId != null) { + UUID.fromString(questionId); + } + return executeWithSql(courseId, feedbackSessionName, questionUuid, selectedSection, fetchType, intent); } else { return executeWithDatastore(courseId, feedbackSessionName, questionId, selectedSection, fetchType, intent); } @@ -165,13 +169,12 @@ private JsonResult executeWithDatastore( } private JsonResult executeWithSql( - String courseId, String feedbackSessionName, String questionId, String selectedSection, + String courseId, String feedbackSessionName, UUID questionUuid, String selectedSection, FeedbackResultFetchType fetchType, Intent intent) { Instructor instructor; Student student; FeedbackSession feedbackSession = getNonNullSqlFeedbackSession(feedbackSessionName, courseId); SqlSessionResultsBundle bundle; - UUID questionUuid = UUID.fromString(questionId); switch (intent) { case FULL_DETAIL: instructor = sqlLogic.getInstructorByGoogleId(courseId, userInfo.id); From 0ad3b56846358352e658e882260631051bc11593 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Tue, 20 Feb 2024 18:20:13 +0800 Subject: [PATCH 19/42] Fix checkstyle errors --- .../sqllogic/core/FeedbackResponseCommentsLogic.java | 4 ++-- .../java/teammates/sqllogic/core/FeedbackResponsesLogic.java | 4 ++-- .../teammates/storage/sqlapi/FeedbackResponseCommentsDb.java | 3 +-- .../java/teammates/storage/sqlapi/FeedbackResponsesDb.java | 1 + 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java b/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java index dc580779f50..31310ed7ef3 100644 --- a/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java +++ b/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java @@ -153,7 +153,7 @@ public void updateFeedbackResponseCommentsForResponse(FeedbackResponse response) * * @param courseId the course ID of the feedback session * @param feedbackSessionName the feedback session name - * @param section if null, will retrieve all comments in the session + * @param sectionName if null, will retrieve all comments in the session * @return a list of feedback response comments */ public List getFeedbackResponseCommentForSessionInSection( @@ -168,7 +168,7 @@ public List getFeedbackResponseCommentForSessionInSecti * Gets all feedback response comments for a question in a section. * * @param questionId the ID of the question - * @param section if null, will retrieve all comments for the question + * @param sectionName if null, will retrieve all comments for the question * @return a list of feedback response comments */ public List getFeedbackResponseCommentForQuestionInSection( diff --git a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java index f2e525732ac..e828034997a 100644 --- a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java +++ b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java @@ -713,7 +713,7 @@ public SqlSessionResultsBundle getSessionResultsForUser( * @param relatedQuestions the relevant questions * @param existingResponses existing responses * @param courseRoster the course roster - * @param section if not null, will only build missing responses for the section + * @param sectionName if not null, will only build missing responses for the section * @return a list of missing responses for the session. */ private List buildMissingResponses( @@ -978,7 +978,7 @@ public List getFeedbackResponsesForQuestion(UUID feedbackQuest * Gets all responses given to/from a section for a question. * * @param feedbackQuestionId the question UUID - * @param section if null, will retrieve all responses for the question + * @param sectionName if null, will retrieve all responses for the question * @return a list of responses */ public List getFeedbackResponsesForQuestionInSection( diff --git a/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java b/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java index e4e15194675..8288b57bcef 100644 --- a/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java +++ b/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java @@ -20,7 +20,6 @@ import jakarta.persistence.criteria.Join; import jakarta.persistence.criteria.ListJoin; import jakarta.persistence.criteria.Root; -import jakarta.persistence.criteria.Subquery; /** * Handles CRUD operations for feedbackResponseComments. @@ -259,7 +258,7 @@ public List getFeedbackResponseCommentsForSessionInSect Join fqJoin = frJoin.join("feedbackQuestion"); Join fsJoin = fqJoin.join("feedbackSession"); Join cJoin = fsJoin.join("course"); - ListJoin sectionsJoin = cJoin.joinList("sections"); + ListJoin sectionsJoin = cJoin.joinList("sections"); cq.select(root) .where(cb.and( diff --git a/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java b/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java index f649865a083..c2c4ae70a26 100644 --- a/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java +++ b/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java @@ -16,6 +16,7 @@ import teammates.storage.sqlentity.FeedbackResponse; import teammates.storage.sqlentity.FeedbackSession; import teammates.storage.sqlentity.Section; + import jakarta.persistence.criteria.CriteriaBuilder; import jakarta.persistence.criteria.CriteriaDelete; import jakarta.persistence.criteria.CriteriaQuery; From a23545d736a972a06431ebe665539b79fe12c6cf Mon Sep 17 00:00:00 2001 From: Xenos F Date: Tue, 20 Feb 2024 18:23:44 +0800 Subject: [PATCH 20/42] Remove unused method parameters --- .../sqllogic/core/FeedbackResponsesLogic.java | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java index e828034997a..db1f657762b 100644 --- a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java +++ b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java @@ -514,7 +514,7 @@ public void updateFeedbackResponsesForChangingSection(Course course, String newE } private List getQuestionsForSession( - FeedbackSession feedbackSession, String courseId, @Nullable UUID questionId) { + FeedbackSession feedbackSession, @Nullable UUID questionId) { if (questionId == null) { return fqLogic.getFeedbackQuestionsForSession(feedbackSession); } @@ -613,8 +613,8 @@ private SqlSessionResultsBundle buildResultsBundle( List missingResponses = Collections.emptyList(); if (isCourseWide) { missingResponses = buildMissingResponses( - courseId, feedbackSession, instructor, responseGiverVisibilityTable, - responseRecipientVisibilityTable, relatedQuestions, existingResponses, roster, sectionName); + instructor, responseGiverVisibilityTable, responseRecipientVisibilityTable, relatedQuestions, + existingResponses, roster, sectionName); } RequestTracer.checkRemainingTime(); @@ -643,7 +643,7 @@ public SqlSessionResultsBundle getSessionResultsForCourse( usersLogic.getInstructorsForCourse(courseId)); // load question(s) - List allQuestions = getQuestionsForSession(feedbackSession, courseId, questionId); + List allQuestions = getQuestionsForSession(feedbackSession, questionId); RequestTracer.checkRemainingTime(); // load response(s) @@ -681,7 +681,7 @@ public SqlSessionResultsBundle getSessionResultsForUser( usersLogic.getInstructorsForCourse(courseId)); // load question(s) - List allQuestions = getQuestionsForSession(feedbackSession, courseId, questionId); + List allQuestions = getQuestionsForSession(feedbackSession, questionId); RequestTracer.checkRemainingTime(); // load response(s) @@ -717,10 +717,8 @@ public SqlSessionResultsBundle getSessionResultsForUser( * @return a list of missing responses for the session. */ private List buildMissingResponses( - String courseId, FeedbackSession feedbackSession, Instructor instructor, - Map responseGiverVisibilityTable, - Map responseRecipientVisibilityTable, - List relatedQuestions, + Instructor instructor, Map responseGiverVisibilityTable, + Map responseRecipientVisibilityTable, List relatedQuestions, List existingResponses, SqlCourseRoster courseRoster, @Nullable String sectionName) { // first get all possible giver recipient pairs From 56a9d931c11e6e0542187f4f3ed80e2af1083646 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Tue, 20 Feb 2024 19:11:55 +0800 Subject: [PATCH 21/42] Fix persistence issues in test cases --- .../ui/webapi/GetSessionResultsActionIT.java | 47 +++++++------------ 1 file changed, 17 insertions(+), 30 deletions(-) diff --git a/src/it/java/teammates/it/ui/webapi/GetSessionResultsActionIT.java b/src/it/java/teammates/it/ui/webapi/GetSessionResultsActionIT.java index d27e094dee4..89dc0703901 100644 --- a/src/it/java/teammates/it/ui/webapi/GetSessionResultsActionIT.java +++ b/src/it/java/teammates/it/ui/webapi/GetSessionResultsActionIT.java @@ -43,6 +43,7 @@ protected void setUp() throws Exception { logoutUser(); persistDataBundle(typicalBundle); HibernateUtil.flushSession(); + HibernateUtil.clearSession(); } @Override @@ -132,31 +133,10 @@ protected void testExecute() { } @Override - protected void testAccessControl() { - // See test cases below - } - - @Test - protected void testAccessControl_invalidIntent_failure() { + protected void testAccessControl() throws Exception { String[] submissionParams; FeedbackSession publishedFeedbackSession = typicalBundle.feedbackSessions.get("session1InCourse1"); - submissionParams = new String[] { - Const.ParamsNames.FEEDBACK_SESSION_NAME, publishedFeedbackSession.getName(), - Const.ParamsNames.COURSE_ID, publishedFeedbackSession.getCourse().getId(), - Const.ParamsNames.INTENT, Intent.INSTRUCTOR_SUBMISSION.name(), - }; - verifyHttpParameterFailure(submissionParams); - submissionParams = new String[] { - Const.ParamsNames.FEEDBACK_SESSION_NAME, publishedFeedbackSession.getName(), - Const.ParamsNames.COURSE_ID, publishedFeedbackSession.getCourse().getId(), - Const.ParamsNames.INTENT, Intent.STUDENT_SUBMISSION.name(), - }; - verifyHttpParameterFailure(submissionParams); - } - - @Test - protected void testAccessControl_resultsNotPublished_shouldNotBeAccessible() { - String[] submissionParams; + Course course = typicalBundle.courses.get("course1"); FeedbackSession inaccessibleFeedbackSession = typicalBundle.feedbackSessions.get( "unpublishedSession1InTypicalCourse"); @@ -177,13 +157,6 @@ protected void testAccessControl_resultsNotPublished_shouldNotBeAccessible() { Student student1InCourse1 = typicalBundle.students.get("student1InCourse1"); loginAsStudent(student1InCourse1.getGoogleId()); verifyCannotAccess(submissionParams); - } - - @Test - protected void testAccessControl_resultsPublished_shouldBeAccessible() throws Exception { - String[] submissionParams; - FeedbackSession publishedFeedbackSession = typicalBundle.feedbackSessions.get("session1InCourse1"); - Course course = publishedFeedbackSession.getCourse(); ______TS("Accessible for authenticated instructor when published"); submissionParams = new String[] { @@ -202,6 +175,20 @@ protected void testAccessControl_resultsPublished_shouldBeAccessible() throws Ex }; verifyAccessibleForStudentsOfTheSameCourse(course, submissionParams); verifyInaccessibleForStudentsOfOtherCourse(course, submissionParams); + + ______TS("Invalid intent"); + submissionParams = new String[] { + Const.ParamsNames.FEEDBACK_SESSION_NAME, publishedFeedbackSession.getName(), + Const.ParamsNames.COURSE_ID, publishedFeedbackSession.getCourse().getId(), + Const.ParamsNames.INTENT, Intent.INSTRUCTOR_SUBMISSION.name(), + }; + verifyHttpParameterFailure(submissionParams); + submissionParams = new String[] { + Const.ParamsNames.FEEDBACK_SESSION_NAME, publishedFeedbackSession.getName(), + Const.ParamsNames.COURSE_ID, publishedFeedbackSession.getCourse().getId(), + Const.ParamsNames.INTENT, Intent.STUDENT_SUBMISSION.name(), + }; + verifyHttpParameterFailure(submissionParams); } private boolean isSessionResultsDataEqual(SessionResultsData self, SessionResultsData other) { From a5de57dd55b865d3e716abcdc206cb2d42d9267d Mon Sep 17 00:00:00 2001 From: Xenos F Date: Fri, 23 Feb 2024 11:52:10 +0800 Subject: [PATCH 22/42] Remove question getter for comment --- .../java/teammates/sqllogic/core/FeedbackResponsesLogic.java | 2 +- .../teammates/storage/sqlentity/FeedbackResponseComment.java | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java index db1f657762b..40c39d036fc 100644 --- a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java +++ b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java @@ -591,7 +591,7 @@ private SqlSessionResultsBundle buildResultsBundle( // build comment for (FeedbackResponseComment frc : allComments) { FeedbackResponse relatedResponse = frc.getFeedbackResponse(); - FeedbackQuestion relatedQuestion = frc.getFeedbackQuestion(); + FeedbackQuestion relatedQuestion = relatedResponse.getFeedbackQuestion(); // the comment needs to be relevant to the question and response if (relatedQuestion == null || relatedResponse == null) { continue; diff --git a/src/main/java/teammates/storage/sqlentity/FeedbackResponseComment.java b/src/main/java/teammates/storage/sqlentity/FeedbackResponseComment.java index aefcb129237..6bd0d8e1df6 100644 --- a/src/main/java/teammates/storage/sqlentity/FeedbackResponseComment.java +++ b/src/main/java/teammates/storage/sqlentity/FeedbackResponseComment.java @@ -111,10 +111,6 @@ public FeedbackResponse getFeedbackResponse() { return feedbackResponse; } - public FeedbackQuestion getFeedbackQuestion() { - return getFeedbackResponse().getFeedbackQuestion(); - } - public void setFeedbackResponse(FeedbackResponse feedbackResponse) { this.feedbackResponse = feedbackResponse; } From 6e62b70bc2319217bd7fa30ab3baf3d0820612c5 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Fri, 23 Feb 2024 11:57:22 +0800 Subject: [PATCH 23/42] Rename boolean methods to start with verb --- .../core/FeedbackResponseCommentsLogic.java | 34 +++++++++---------- .../sqllogic/core/FeedbackResponsesLogic.java | 5 +-- .../sqlentity/FeedbackResponseComment.java | 2 +- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java b/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java index 31310ed7ef3..7883c7e5432 100644 --- a/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java +++ b/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java @@ -183,7 +183,7 @@ public List getFeedbackResponseCommentForQuestionInSect * Verifies whether the comment is visible to certain user. * @return true/false */ - public boolean isResponseCommentVisibleForUser(String userEmail, boolean isInstructor, + public boolean checkIsResponseCommentVisibleForUser(String userEmail, boolean isInstructor, Student student, Set studentsEmailInTeam, FeedbackResponse response, FeedbackQuestion relatedQuestion, FeedbackResponseComment relatedComment) { @@ -193,37 +193,37 @@ public boolean isResponseCommentVisibleForUser(String userEmail, boolean isInstr boolean isVisibilityFollowingFeedbackQuestion = relatedComment.getIsVisibilityFollowingFeedbackQuestion(); boolean isVisibleToGiver = isVisibilityFollowingFeedbackQuestion - || relatedComment.isVisibleTo(FeedbackParticipantType.GIVER); + || relatedComment.checkIsVisibleTo(FeedbackParticipantType.GIVER); - boolean isVisibleToUser = isVisibleToUser(userEmail, response, relatedQuestion, relatedComment, + boolean isVisibleToUser = checkIsVisibleToUser(userEmail, response, relatedQuestion, relatedComment, isVisibleToGiver, isInstructor, !isInstructor); - boolean isVisibleToUserTeam = isVisibleToUserTeam(student, studentsEmailInTeam, response, + boolean isVisibleToUserTeam = checkIsVisibleToUserTeam(student, studentsEmailInTeam, response, relatedQuestion, relatedComment, !isInstructor); return isVisibleToUser || isVisibleToUserTeam; } - private boolean isVisibleToUserTeam(Student student, Set studentsEmailInTeam, + private boolean checkIsVisibleToUserTeam(Student student, Set studentsEmailInTeam, FeedbackResponse response, FeedbackQuestion relatedQuestion, FeedbackResponseComment relatedComment, boolean isUserStudent) { boolean isUserInResponseRecipientTeamAndRelatedResponseCommentVisibleToRecipients = isUserStudent && relatedQuestion.getRecipientType() == FeedbackParticipantType.TEAMS - && isResponseCommentVisibleTo(relatedQuestion, relatedComment, + && checkIsResponseCommentVisibleTo(relatedQuestion, relatedComment, FeedbackParticipantType.RECEIVER) && response.getRecipient().equals(student.getTeamName()); boolean isUserInResponseGiverTeamAndRelatedResponseCommentVisibleToGiversTeamMembers = (relatedQuestion.getGiverType() == FeedbackParticipantType.TEAMS - || isResponseCommentVisibleTo(relatedQuestion, relatedComment, + || checkIsResponseCommentVisibleTo(relatedQuestion, relatedComment, FeedbackParticipantType.OWN_TEAM_MEMBERS)) && (studentsEmailInTeam.contains(response.getGiver()) || isUserStudent && student.getTeamName().equals(response.getGiver())); boolean isUserInResponseRecipientTeamAndRelatedResponseCommentVisibleToRecipientsTeamMembers = - isResponseCommentVisibleTo(relatedQuestion, relatedComment, + checkIsResponseCommentVisibleTo(relatedQuestion, relatedComment, FeedbackParticipantType.RECEIVER_TEAM_MEMBERS) && studentsEmailInTeam.contains(response.getRecipient()); @@ -232,16 +232,16 @@ && isResponseCommentVisibleTo(relatedQuestion, relatedComment, || isUserInResponseRecipientTeamAndRelatedResponseCommentVisibleToRecipientsTeamMembers; } - private boolean isVisibleToUser(String userEmail, FeedbackResponse response, + private boolean checkIsVisibleToUser(String userEmail, FeedbackResponse response, FeedbackQuestion relatedQuestion, FeedbackResponseComment relatedComment, boolean isVisibleToGiver, boolean isUserInstructor, boolean isUserStudent) { boolean isUserInstructorAndRelatedResponseCommentVisibleToInstructors = - isUserInstructor && isResponseCommentVisibleTo(relatedQuestion, relatedComment, + isUserInstructor && checkIsResponseCommentVisibleTo(relatedQuestion, relatedComment, FeedbackParticipantType.INSTRUCTORS); boolean isUserResponseRecipientAndRelatedResponseCommentVisibleToRecipients = - response.getRecipient().equals(userEmail) && isResponseCommentVisibleTo(relatedQuestion, + response.getRecipient().equals(userEmail) && checkIsResponseCommentVisibleTo(relatedQuestion, relatedComment, FeedbackParticipantType.RECEIVER); boolean isUserResponseGiverAndRelatedResponseCommentVisibleToGivers = @@ -250,7 +250,7 @@ isUserInstructor && isResponseCommentVisibleTo(relatedQuestion, relatedComment, boolean isUserRelatedResponseCommentGiver = relatedComment.getGiver().equals(userEmail); boolean isUserStudentAndRelatedResponseCommentVisibleToStudents = - isUserStudent && isResponseCommentVisibleTo(relatedQuestion, + isUserStudent && checkIsResponseCommentVisibleTo(relatedQuestion, relatedComment, FeedbackParticipantType.STUDENTS); return isUserInstructorAndRelatedResponseCommentVisibleToInstructors @@ -260,19 +260,19 @@ isUserStudent && isResponseCommentVisibleTo(relatedQuestion, || isUserStudentAndRelatedResponseCommentVisibleToStudents; } - private boolean isResponseCommentVisibleTo(FeedbackQuestion relatedQuestion, + private boolean checkIsResponseCommentVisibleTo(FeedbackQuestion relatedQuestion, FeedbackResponseComment relatedComment, FeedbackParticipantType viewerType) { boolean isVisibilityFollowingFeedbackQuestion = relatedComment.getIsVisibilityFollowingFeedbackQuestion(); return isVisibilityFollowingFeedbackQuestion ? relatedQuestion.isResponseVisibleTo(viewerType) - : relatedComment.isVisibleTo(viewerType); + : relatedComment.checkIsVisibleTo(viewerType); } /** * Returns true if the comment's giver name is visible to certain user. */ - public boolean isNameVisibleToUser(FeedbackResponseComment comment, FeedbackResponse response, + public boolean checkIsNameVisibleToUser(FeedbackResponseComment comment, FeedbackResponse response, String userEmail, SqlCourseRoster roster) { List showNameTo = comment.getShowGiverNameTo(); //in the old ver, name is always visible @@ -285,10 +285,10 @@ public boolean isNameVisibleToUser(FeedbackResponseComment comment, FeedbackResp return true; } - return isFeedbackParticipantNameVisibleToUser(response, userEmail, roster, showNameTo); + return checkIsFeedbackParticipantNameVisibleToUser(response, userEmail, roster, showNameTo); } - private boolean isFeedbackParticipantNameVisibleToUser(FeedbackResponse response, + private boolean checkIsFeedbackParticipantNameVisibleToUser(FeedbackResponse response, String userEmail, SqlCourseRoster roster, List showNameTo) { String responseGiverTeam = "giverTeam"; if (roster.getStudentForEmail(response.getGiver()) != null) { diff --git a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java index 40c39d036fc..c5cd764ebfe 100644 --- a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java +++ b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java @@ -597,7 +597,7 @@ private SqlSessionResultsBundle buildResultsBundle( continue; } // check visibility of comment - boolean isVisibleResponseComment = frcLogic.isResponseCommentVisibleForUser( + boolean isVisibleResponseComment = frcLogic.checkIsResponseCommentVisibleForUser( userEmail, isInstructor, student, studentsEmailInTeam, relatedResponse, relatedQuestion, frc); if (!isVisibleResponseComment) { continue; @@ -605,7 +605,8 @@ private SqlSessionResultsBundle buildResultsBundle( relatedCommentsMap.computeIfAbsent(relatedResponse, key -> new ArrayList<>()).add(frc); // generate comment giver name visibility table - commentVisibilityTable.put(frc.getId(), frcLogic.isNameVisibleToUser(frc, relatedResponse, userEmail, roster)); + commentVisibilityTable.put(frc.getId(), + frcLogic.checkIsNameVisibleToUser(frc, relatedResponse, userEmail, roster)); } RequestTracer.checkRemainingTime(); diff --git a/src/main/java/teammates/storage/sqlentity/FeedbackResponseComment.java b/src/main/java/teammates/storage/sqlentity/FeedbackResponseComment.java index 6bd0d8e1df6..29f3824dbba 100644 --- a/src/main/java/teammates/storage/sqlentity/FeedbackResponseComment.java +++ b/src/main/java/teammates/storage/sqlentity/FeedbackResponseComment.java @@ -214,7 +214,7 @@ public void sanitizeForSaving() { /** * Returns true if the response comment is visible to the given participant type. */ - public boolean isVisibleTo(FeedbackParticipantType viewerType) { + public boolean checkIsVisibleTo(FeedbackParticipantType viewerType) { return showCommentTo.contains(viewerType); } From fa895599b0fada1043311a3b2ecc95b97e89ef0b Mon Sep 17 00:00:00 2001 From: Xenos F Date: Fri, 23 Feb 2024 15:07:23 +0800 Subject: [PATCH 24/42] Reword comment to clarify question ID --- src/main/java/teammates/ui/webapi/GetSessionResultsAction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java b/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java index 785c911b32e..29b28c813ba 100644 --- a/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java +++ b/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java @@ -107,7 +107,7 @@ public JsonResult execute() { String courseId = getNonNullRequestParamValue(Const.ParamsNames.COURSE_ID); String feedbackSessionName = getNonNullRequestParamValue(Const.ParamsNames.FEEDBACK_SESSION_NAME); - // Allow additional filter by question ID (equivalent to question number) and section name + // Allow additional filter by question ID and section name String questionId = getRequestParamValue(Const.ParamsNames.FEEDBACK_QUESTION_ID); String selectedSection = getRequestParamValue(Const.ParamsNames.FEEDBACK_RESULTS_GROUPBYSECTION); FeedbackResultFetchType fetchType = FeedbackResultFetchType.parseFetchType( From 273032b9f8f09d755b4c085b3255104d545c2bae Mon Sep 17 00:00:00 2001 From: Xenos F Date: Fri, 23 Feb 2024 15:08:18 +0800 Subject: [PATCH 25/42] Refactor getting question UUID from param value --- .../java/teammates/ui/webapi/GetSessionResultsAction.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java b/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java index 29b28c813ba..13347f91f60 100644 --- a/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java +++ b/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java @@ -116,11 +116,12 @@ public JsonResult execute() { Intent intent = Intent.valueOf(getNonNullRequestParamValue(Const.ParamsNames.INTENT)); if (isCourseMigrated(courseId)) { - UUID questionUuid = null; if (questionId != null) { - UUID.fromString(questionId); + UUID questionUuid = getUuidRequestParamValue(Const.ParamsNames.FEEDBACK_QUESTION_ID); + executeWithSql(courseId, feedbackSessionName, questionUuid, + selectedSection, fetchType, intent); } - return executeWithSql(courseId, feedbackSessionName, questionUuid, selectedSection, fetchType, intent); + return executeWithSql(courseId, feedbackSessionName, null, selectedSection, fetchType, intent); } else { return executeWithDatastore(courseId, feedbackSessionName, questionId, selectedSection, fetchType, intent); } From 8627c3feb432fe8fa04b55cd94a1b4c5bdcc60b6 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Fri, 23 Feb 2024 16:28:23 +0800 Subject: [PATCH 26/42] Remove unneeded getters --- .../teammates/sqllogic/core/FeedbackResponsesLogic.java | 4 ++-- .../teammates/storage/sqlentity/FeedbackResponse.java | 8 -------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java index c5cd764ebfe..d3bbcf6c428 100644 --- a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java +++ b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java @@ -921,14 +921,14 @@ private boolean isResponseVisibleForUser( if (isVisibleResponse && instructor != null) { boolean isGiverSectionRestricted = !instructor.isAllowedForPrivilege(response.getGiverSection().getName(), - response.getFeedbackSessionName(), + response.getFeedbackQuestion().getFeedbackSession().getName(), Const.InstructorPermissions.CAN_VIEW_SESSION_IN_SECTIONS); // If instructors are not restricted to view the giver's section, // they are allowed to view responses to GENERAL, subject to visibility options boolean isRecipientSectionRestricted = relatedQuestion.getRecipientType() != FeedbackParticipantType.NONE && !instructor.isAllowedForPrivilege(response.getRecipientSection().getName(), - response.getFeedbackSessionName(), + response.getFeedbackQuestion().getFeedbackSession().getName(), Const.InstructorPermissions.CAN_VIEW_SESSION_IN_SECTIONS); boolean isNotAllowedForInstructor = isGiverSectionRestricted || isRecipientSectionRestricted; diff --git a/src/main/java/teammates/storage/sqlentity/FeedbackResponse.java b/src/main/java/teammates/storage/sqlentity/FeedbackResponse.java index 87ef587a6e0..40caa7f7c36 100644 --- a/src/main/java/teammates/storage/sqlentity/FeedbackResponse.java +++ b/src/main/java/teammates/storage/sqlentity/FeedbackResponse.java @@ -232,14 +232,6 @@ public void setUpdatedAt(Instant updatedAt) { this.updatedAt = updatedAt; } - public FeedbackSession getFeedbackSession() { - return feedbackQuestion.getFeedbackSession(); - } - - public String getFeedbackSessionName() { - return getFeedbackSession().getName(); - } - @Override public List getInvalidityInfo() { return new ArrayList<>(); From 7f7b81b1393db472f7a1e5c6c4440865cb3f9918 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Fri, 23 Feb 2024 16:30:50 +0800 Subject: [PATCH 27/42] Remove entities from Const --- src/main/java/teammates/common/util/Const.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/main/java/teammates/common/util/Const.java b/src/main/java/teammates/common/util/Const.java index 8987d6f1cb6..b4c8c1cd259 100644 --- a/src/main/java/teammates/common/util/Const.java +++ b/src/main/java/teammates/common/util/Const.java @@ -6,7 +6,6 @@ import java.time.Instant; import teammates.storage.sqlentity.Section; -import teammates.storage.sqlentity.Team; /** * Stores constants that are widely used across classes. @@ -20,7 +19,6 @@ public final class Const { public static final String USER_NOBODY_TEXT = "-"; public static final String USER_TEAM_FOR_INSTRUCTOR = "Instructors"; - public static final Team USER_TEAM_ENTITY_FOR_INSTRUCTOR = new Team(null, Const.USER_TEAM_FOR_INSTRUCTOR); public static final String DEFAULT_DISPLAY_NAME_FOR_INSTRUCTOR = "Instructor"; @@ -29,7 +27,6 @@ public final class Const { public static final int SECTION_SIZE_LIMIT = 100; public static final String DEFAULT_SECTION = "None"; - public static final Section DEFAULT_SECTION_ENTITY = new Section(null, DEFAULT_SECTION); public static final Section DEFAULT_SQL_SECTION = null; public static final String UNKNOWN_INSTITUTION = "Unknown Institution"; From b6d4b0f49f5ad17981a4d6704235786c581a550f Mon Sep 17 00:00:00 2001 From: Xenos F Date: Fri, 23 Feb 2024 16:40:19 +0800 Subject: [PATCH 28/42] Revert changes to SqlCourseRoster --- .../common/datatransfer/SqlCourseRoster.java | 42 +++++++------------ 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/src/main/java/teammates/common/datatransfer/SqlCourseRoster.java b/src/main/java/teammates/common/datatransfer/SqlCourseRoster.java index 83b8021cfb2..922b9d6c8c6 100644 --- a/src/main/java/teammates/common/datatransfer/SqlCourseRoster.java +++ b/src/main/java/teammates/common/datatransfer/SqlCourseRoster.java @@ -7,9 +7,7 @@ import teammates.common.util.Const; import teammates.storage.sqlentity.Instructor; -import teammates.storage.sqlentity.Section; import teammates.storage.sqlentity.Student; -import teammates.storage.sqlentity.Team; /** * Contains a list of students and instructors in a course. Useful for caching @@ -128,8 +126,8 @@ public static Map> buildTeamToMembersTable(List s */ public ParticipantInfo getInfoForIdentifier(String identifier) { String name = Const.USER_NOBODY_TEXT; - Team team = new Team(null, Const.USER_NOBODY_TEXT); - Section section = Const.DEFAULT_SECTION_ENTITY; + String teamName = Const.USER_NOBODY_TEXT; + String sectionName = Const.DEFAULT_SECTION; boolean isStudent = getStudentForEmail(identifier) != null; boolean isInstructor = getInstructorForEmail(identifier) != null; @@ -138,23 +136,23 @@ public ParticipantInfo getInfoForIdentifier(String identifier) { Student student = getStudentForEmail(identifier); name = student.getName(); - team = student.getTeam(); - section = student.getSection(); + teamName = student.getTeamName(); + sectionName = student.getSectionName(); } else if (isInstructor) { Instructor instructor = getInstructorForEmail(identifier); name = instructor.getName(); - team = Const.USER_TEAM_ENTITY_FOR_INSTRUCTOR; - section = Const.DEFAULT_SECTION_ENTITY; + teamName = Const.USER_TEAM_FOR_INSTRUCTOR; + sectionName = Const.DEFAULT_SECTION; } else if (isTeam) { Student teamMember = getTeamToMembersTable().get(identifier).iterator().next(); name = identifier; - team = new Team(teamMember.getSection(), identifier); - section = teamMember.getSection(); + teamName = identifier; + sectionName = teamMember.getSectionName(); } - return new ParticipantInfo(name, team, section); + return new ParticipantInfo(name, teamName, sectionName); } /** @@ -163,33 +161,25 @@ public ParticipantInfo getInfoForIdentifier(String identifier) { public static class ParticipantInfo { private final String name; - private final Team team; - private final Section section; + private final String teamName; + private final String sectionName; - private ParticipantInfo(String name, Team team, Section section) { + private ParticipantInfo(String name, String teamName, String sectionName) { this.name = name; - this.team = team; - this.section = section; + this.teamName = teamName; + this.sectionName = sectionName; } public String getName() { return name; } - public Team getTeam() { - return team; - } - - public Section getSection() { - return section; - } - public String getTeamName() { - return team.getName(); + return teamName; } public String getSectionName() { - return section.getName(); + return sectionName; } } } From bd67d3cd172471ed09f3dbe67792f551138de4af Mon Sep 17 00:00:00 2001 From: Xenos F Date: Fri, 23 Feb 2024 17:08:16 +0800 Subject: [PATCH 29/42] Create and use missing response class --- .../sqllogic/core/FeedbackResponsesLogic.java | 23 ++++++------ .../storage/sqlentity/FeedbackResponse.java | 8 +++++ .../responses/FeedbackMissingResponse.java | 36 +++++++++++++++++++ .../ui/output/SessionResultsData.java | 4 +-- 4 files changed, 57 insertions(+), 14 deletions(-) create mode 100644 src/main/java/teammates/storage/sqlentity/responses/FeedbackMissingResponse.java diff --git a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java index d3bbcf6c428..51aa0bd6a09 100644 --- a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java +++ b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java @@ -17,7 +17,6 @@ import teammates.common.datatransfer.SqlSessionResultsBundle; import teammates.common.datatransfer.questions.FeedbackQuestionType; import teammates.common.datatransfer.questions.FeedbackRankRecipientsResponseDetails; -import teammates.common.datatransfer.questions.FeedbackTextResponseDetails; import teammates.common.exception.EntityAlreadyExistsException; import teammates.common.exception.EntityDoesNotExistException; import teammates.common.exception.InvalidParametersException; @@ -33,6 +32,7 @@ import teammates.storage.sqlentity.Section; import teammates.storage.sqlentity.Student; import teammates.storage.sqlentity.Team; +import teammates.storage.sqlentity.responses.FeedbackMissingResponse; import teammates.storage.sqlentity.responses.FeedbackRankRecipientsResponse; /** @@ -591,9 +591,12 @@ private SqlSessionResultsBundle buildResultsBundle( // build comment for (FeedbackResponseComment frc : allComments) { FeedbackResponse relatedResponse = frc.getFeedbackResponse(); - FeedbackQuestion relatedQuestion = relatedResponse.getFeedbackQuestion(); // the comment needs to be relevant to the question and response - if (relatedQuestion == null || relatedResponse == null) { + if (relatedResponse == null) { + continue; + } + FeedbackQuestion relatedQuestion = relatedResponse.getFeedbackQuestion(); + if (relatedQuestion == null) { continue; } // check visibility of comment @@ -766,14 +769,10 @@ private List buildMissingResponses( continue; } - FeedbackResponse missingResponse = FeedbackResponse.makeResponse( + FeedbackResponse missingResponse = new FeedbackMissingResponse( correspondingQuestion, - giverIdentifier, - giverInfo.getSection(), - recipientIdentifier, - recipientInfo.getSection(), - new FeedbackTextResponseDetails("No Response") - ); + giverIdentifier, giverInfo.getSectionName(), + recipientIdentifier, recipientInfo.getSectionName()); // check visibility of the missing response boolean isVisibleResponse = isResponseVisibleForUser( @@ -920,14 +919,14 @@ private boolean isResponseVisibleForUser( } if (isVisibleResponse && instructor != null) { boolean isGiverSectionRestricted = - !instructor.isAllowedForPrivilege(response.getGiverSection().getName(), + !instructor.isAllowedForPrivilege(response.getGiverSectionName(), response.getFeedbackQuestion().getFeedbackSession().getName(), Const.InstructorPermissions.CAN_VIEW_SESSION_IN_SECTIONS); // If instructors are not restricted to view the giver's section, // they are allowed to view responses to GENERAL, subject to visibility options boolean isRecipientSectionRestricted = relatedQuestion.getRecipientType() != FeedbackParticipantType.NONE - && !instructor.isAllowedForPrivilege(response.getRecipientSection().getName(), + && !instructor.isAllowedForPrivilege(response.getRecipientSectionName(), response.getFeedbackQuestion().getFeedbackSession().getName(), Const.InstructorPermissions.CAN_VIEW_SESSION_IN_SECTIONS); diff --git a/src/main/java/teammates/storage/sqlentity/FeedbackResponse.java b/src/main/java/teammates/storage/sqlentity/FeedbackResponse.java index 40caa7f7c36..eb895e3d9be 100644 --- a/src/main/java/teammates/storage/sqlentity/FeedbackResponse.java +++ b/src/main/java/teammates/storage/sqlentity/FeedbackResponse.java @@ -204,6 +204,10 @@ public Section getGiverSection() { return giverSection; } + public String getGiverSectionName() { + return giverSection.getName(); + } + public void setGiverSection(Section giverSection) { this.giverSection = giverSection; } @@ -220,6 +224,10 @@ public Section getRecipientSection() { return recipientSection; } + public String getRecipientSectionName() { + return recipientSection.getName(); + } + public void setRecipientSection(Section recipientSection) { this.recipientSection = recipientSection; } diff --git a/src/main/java/teammates/storage/sqlentity/responses/FeedbackMissingResponse.java b/src/main/java/teammates/storage/sqlentity/responses/FeedbackMissingResponse.java new file mode 100644 index 00000000000..d8be7795d0c --- /dev/null +++ b/src/main/java/teammates/storage/sqlentity/responses/FeedbackMissingResponse.java @@ -0,0 +1,36 @@ +package teammates.storage.sqlentity.responses; + +import teammates.common.datatransfer.questions.FeedbackTextResponseDetails; +import teammates.storage.sqlentity.FeedbackQuestion; + +/** + * Represents a missing response. + */ +public class FeedbackMissingResponse extends FeedbackTextResponse { + + private String giverSectionName; + private String recipientSectionName; + + protected FeedbackMissingResponse() { + // required by Hibernate + } + + public FeedbackMissingResponse( + FeedbackQuestion feedbackQuestion, String giver, + String giverSectionName, String recipient, String recipientSectionName + ) { + super(feedbackQuestion, giver, null, recipient, null, new FeedbackTextResponseDetails("No Response")); + this.giverSectionName = giverSectionName; + this.recipientSectionName = recipientSectionName; + } + + @Override + public String getGiverSectionName() { + return giverSectionName; + } + + @Override + public String getRecipientSectionName() { + return recipientSectionName; + } +} diff --git a/src/main/java/teammates/ui/output/SessionResultsData.java b/src/main/java/teammates/ui/output/SessionResultsData.java index dfea2e0ef77..c504c6aef1b 100644 --- a/src/main/java/teammates/ui/output/SessionResultsData.java +++ b/src/main/java/teammates/ui/output/SessionResultsData.java @@ -467,7 +467,7 @@ private static ResponseOutput buildSingleResponseForInstructor( } String giverName = getGiverNameOfResponse(response, bundle); String giverTeam = bundle.getRoster().getInfoForIdentifier(response.getGiver()).getTeamName(); - String giverSectionName = response.getGiverSection().getName(); + String giverSectionName = response.getGiverSectionName(); FeedbackQuestion question = response.getFeedbackQuestion(); if (question.getGiverType() == FeedbackParticipantType.INSTRUCTORS) { Instructor instructor = bundle.getRoster().getInstructorForEmail(response.getGiver()); @@ -481,7 +481,7 @@ private static ResponseOutput buildSingleResponseForInstructor( String recipientName = getRecipientNameOfResponse(response, bundle); String recipientTeam = bundle.getRoster().getInfoForIdentifier(response.getRecipient()).getTeamName(); - String recipientSectionName = response.getRecipientSection().getName(); + String recipientSectionName = response.getRecipientSectionName(); if (question.getRecipientType() == FeedbackParticipantType.INSTRUCTORS) { Instructor instructor = bundle.getRoster().getInstructorForEmail(response.getRecipient()); recipientName = instructor.getName(); From 0668931ae77eb2dac7c40741a5d39db092721931 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Sat, 24 Feb 2024 09:27:37 +0800 Subject: [PATCH 30/42] Refactor no response text to const --- src/main/java/teammates/common/util/Const.java | 2 ++ .../storage/sqlentity/responses/FeedbackMissingResponse.java | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/java/teammates/common/util/Const.java b/src/main/java/teammates/common/util/Const.java index 155cbd7fb59..ebda96222ae 100644 --- a/src/main/java/teammates/common/util/Const.java +++ b/src/main/java/teammates/common/util/Const.java @@ -43,6 +43,8 @@ public final class Const { public static final String ERROR_CREATE_ENTITY_ALREADY_EXISTS = "Trying to create an entity that exists: %s"; public static final String ERROR_UPDATE_NON_EXISTENT = "Trying to update non-existent Entity: "; + public static final String MISSING_RESPONSE_TEXT = "No Response"; + // These constants are used as variable values to mean that the variable is in a 'special' state. public static final int INT_UNINITIALIZED = -9999; diff --git a/src/main/java/teammates/storage/sqlentity/responses/FeedbackMissingResponse.java b/src/main/java/teammates/storage/sqlentity/responses/FeedbackMissingResponse.java index d8be7795d0c..fd9e4c85b13 100644 --- a/src/main/java/teammates/storage/sqlentity/responses/FeedbackMissingResponse.java +++ b/src/main/java/teammates/storage/sqlentity/responses/FeedbackMissingResponse.java @@ -1,6 +1,7 @@ package teammates.storage.sqlentity.responses; import teammates.common.datatransfer.questions.FeedbackTextResponseDetails; +import teammates.common.util.Const; import teammates.storage.sqlentity.FeedbackQuestion; /** @@ -19,7 +20,7 @@ public FeedbackMissingResponse( FeedbackQuestion feedbackQuestion, String giver, String giverSectionName, String recipient, String recipientSectionName ) { - super(feedbackQuestion, giver, null, recipient, null, new FeedbackTextResponseDetails("No Response")); + super(feedbackQuestion, giver, null, recipient, null, new FeedbackTextResponseDetails(Const.MISSING_RESPONSE_TEXT)); this.giverSectionName = giverSectionName; this.recipientSectionName = recipientSectionName; } From a012ad20f0dd2bac4c2b2d59790835118d3b7ebe Mon Sep 17 00:00:00 2001 From: Xenos F Date: Sat, 24 Feb 2024 11:06:26 +0800 Subject: [PATCH 31/42] Migrate preview-related functionality --- .../datatransfer/SqlSessionResultsBundle.java | 15 +++++ .../java/teammates/sqllogic/api/Logic.java | 4 +- .../sqllogic/core/FeedbackResponsesLogic.java | 65 +++++++++++++++++-- .../ui/webapi/GetSessionResultsAction.java | 6 +- .../SqlSessionResultsBundleTest.java | 9 +++ 5 files changed, 88 insertions(+), 11 deletions(-) diff --git a/src/main/java/teammates/common/datatransfer/SqlSessionResultsBundle.java b/src/main/java/teammates/common/datatransfer/SqlSessionResultsBundle.java index 9521b4b94d6..8bc53624d26 100644 --- a/src/main/java/teammates/common/datatransfer/SqlSessionResultsBundle.java +++ b/src/main/java/teammates/common/datatransfer/SqlSessionResultsBundle.java @@ -4,6 +4,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import teammates.common.util.Const; import teammates.common.util.StringHelper; @@ -17,6 +18,8 @@ public class SqlSessionResultsBundle { private final List questions; + private final Set questionsNotVisibleForPreviewSet; + private final Set questionsWithCommentNotVisibleForPreviewSet; private final Map> questionResponseMap; private final Map> questionMissingResponseMap; private final Map> responseCommentsMap; @@ -26,6 +29,8 @@ public class SqlSessionResultsBundle { private final SqlCourseRoster roster; public SqlSessionResultsBundle(List questions, + Set questionsNotVisibleForPreviewSet, + Set questionsWithCommentNotVisibleForPreviewSet, List responses, List missingResponses, Map responseGiverVisibilityTable, @@ -35,6 +40,8 @@ public SqlSessionResultsBundle(List questions, SqlCourseRoster roster) { this.questions = questions; + this.questionsNotVisibleForPreviewSet = questionsNotVisibleForPreviewSet; + this.questionsWithCommentNotVisibleForPreviewSet = questionsWithCommentNotVisibleForPreviewSet; this.responseCommentsMap = responseCommentsMap; this.responseGiverVisibilityTable = responseGiverVisibilityTable; this.responseRecipientVisibilityTable = responseRecipientVisibilityTable; @@ -154,4 +161,12 @@ public Map getResponseRecipientVisibilityTable() { public Map getCommentGiverVisibilityTable() { return commentGiverVisibilityTable; } + + public Set getQuestionsNotVisibleForPreviewSet() { + return questionsNotVisibleForPreviewSet; + } + + public Set getQuestionsWithCommentNotVisibleForPreviewSet() { + return questionsWithCommentNotVisibleForPreviewSet; + } } diff --git a/src/main/java/teammates/sqllogic/api/Logic.java b/src/main/java/teammates/sqllogic/api/Logic.java index 28230951184..3d3cd21810c 100644 --- a/src/main/java/teammates/sqllogic/api/Logic.java +++ b/src/main/java/teammates/sqllogic/api/Logic.java @@ -1231,13 +1231,13 @@ public SqlSessionResultsBundle getSessionResultsForCourse( */ public SqlSessionResultsBundle getSessionResultsForUser( FeedbackSession feedbackSession, String courseId, String userEmail, boolean isInstructor, - @Nullable UUID questionId) { + @Nullable UUID questionId, boolean isPreviewResults) { assert feedbackSession != null; assert courseId != null; assert userEmail != null; return feedbackResponsesLogic.getSessionResultsForUser( - feedbackSession, courseId, userEmail, isInstructor, questionId); + feedbackSession, courseId, userEmail, isInstructor, questionId, isPreviewResults); } /** diff --git a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java index 51aa0bd6a09..ca06cffccaa 100644 --- a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java +++ b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java @@ -526,7 +526,16 @@ private SqlSessionResultsBundle buildResultsBundle( boolean isCourseWide, FeedbackSession feedbackSession, String courseId, String sectionName, UUID questionId, boolean isInstructor, String userEmail, Instructor instructor, Student student, SqlCourseRoster roster, List allQuestions, - List allResponses) { + List allResponses, boolean isPreviewResults) { + + Set questionsNotVisibleToInstructors = new HashSet<>(); + for (FeedbackQuestion qn : allQuestions) { + + // set questions that should not be visible to instructors if results are being previewed + if (isPreviewResults && !checkCanInstructorsSeeQuestion(qn)) { + questionsNotVisibleToInstructors.add(qn); + } + } // load comment(s) List allComments; @@ -561,9 +570,16 @@ private SqlSessionResultsBundle buildResultsBundle( Map responseGiverVisibilityTable = new HashMap<>(); Map responseRecipientVisibilityTable = new HashMap<>(); Map commentVisibilityTable = new HashMap<>(); + Set relatedQuestionsNotVisibleForPreviewSet = new HashSet<>(); + Set relatedQuestionsWithCommentNotVisibleForPreview = new HashSet<>(); // build response for (FeedbackResponse response : allResponses) { + if (isPreviewResults + && relatedQuestionsNotVisibleForPreviewSet.contains(response.getFeedbackQuestion())) { + // corresponding question's responses will not be shown to previewer, ignore the response + continue; + } FeedbackQuestion correspondingQuestion = response.getFeedbackQuestion(); if (correspondingQuestion == null) { // orphan response without corresponding question, ignore it @@ -576,6 +592,13 @@ private SqlSessionResultsBundle buildResultsBundle( continue; } + // if previewing results and corresponding question should not be visible to instructors, + // note down the question and do not add the response + if (isPreviewResults && questionsNotVisibleToInstructors.contains(response.getFeedbackQuestion())) { + relatedQuestionsNotVisibleForPreviewSet.add(response.getFeedbackQuestion()); + continue; + } + // if there are viewable responses, the corresponding question becomes related relatedQuestions.add(response.getFeedbackQuestion()); relatedResponses.add(response); @@ -606,6 +629,13 @@ private SqlSessionResultsBundle buildResultsBundle( continue; } + // if previewing results and the comment should not be visible to instructors, + // note down the corresponding question and do not add the comment + if (isPreviewResults && !checkCanInstructorsSeeComment(frc)) { + relatedQuestionsWithCommentNotVisibleForPreview.add(frc.getFeedbackResponse().getFeedbackQuestion()); + continue; + } + relatedCommentsMap.computeIfAbsent(relatedResponse, key -> new ArrayList<>()).add(frc); // generate comment giver name visibility table commentVisibilityTable.put(frc.getId(), @@ -622,7 +652,8 @@ private SqlSessionResultsBundle buildResultsBundle( } RequestTracer.checkRemainingTime(); - return new SqlSessionResultsBundle(relatedQuestions, existingResponses, missingResponses, + return new SqlSessionResultsBundle(relatedQuestions, relatedQuestionsNotVisibleForPreviewSet, + relatedQuestionsWithCommentNotVisibleForPreview, existingResponses, missingResponses, responseGiverVisibilityTable, responseRecipientVisibilityTable, relatedCommentsMap, commentVisibilityTable, roster); } @@ -664,7 +695,7 @@ public SqlSessionResultsBundle getSessionResultsForCourse( Instructor instructor = usersLogic.getInstructorForEmail(courseId, instructorEmail); return buildResultsBundle(true, feedbackSession, courseId, sectionName, questionId, true, instructorEmail, - instructor, null, roster, allQuestions, allResponses); + instructor, null, roster, allQuestions, allResponses, false); } /** @@ -679,7 +710,7 @@ public SqlSessionResultsBundle getSessionResultsForCourse( */ public SqlSessionResultsBundle getSessionResultsForUser( FeedbackSession feedbackSession, String courseId, String userEmail, boolean isInstructor, - @Nullable UUID questionId) { + @Nullable UUID questionId, boolean isPreviewResults) { SqlCourseRoster roster = new SqlCourseRoster( usersLogic.getStudentsForCourse(courseId), usersLogic.getInstructorsForCourse(courseId)); @@ -703,7 +734,7 @@ public SqlSessionResultsBundle getSessionResultsForUser( RequestTracer.checkRemainingTime(); return buildResultsBundle(false, feedbackSession, courseId, null, questionId, isInstructor, userEmail, - instructor, student, roster, allQuestions, allResponses); + instructor, student, roster, allQuestions, allResponses, isPreviewResults); } /** @@ -1078,4 +1109,28 @@ private List getFeedbackResponsesForRecipientForQuestion( return frDb.getFeedbackResponsesForRecipientForQuestion(feedbackQuestionId, userEmail); } + /** + * Checks whether instructors can see the question. + */ + boolean checkCanInstructorsSeeQuestion(FeedbackQuestion feedbackQuestion) { + boolean isResponseVisibleToInstructor = + feedbackQuestion.getShowResponsesTo().contains(FeedbackParticipantType.INSTRUCTORS); + boolean isGiverVisibleToInstructor = + feedbackQuestion.getShowGiverNameTo().contains(FeedbackParticipantType.INSTRUCTORS); + boolean isRecipientVisibleToInstructor = + feedbackQuestion.getShowRecipientNameTo().contains(FeedbackParticipantType.INSTRUCTORS); + return isResponseVisibleToInstructor && isGiverVisibleToInstructor && isRecipientVisibleToInstructor; + } + + /** + * Checks whether instructors can see the comment. + */ + boolean checkCanInstructorsSeeComment(FeedbackResponseComment feedbackResponseComment) { + boolean isCommentVisibleToInstructor = + feedbackResponseComment.getShowCommentTo().contains(FeedbackParticipantType.INSTRUCTORS); + boolean isGiverVisibleToInstructor = + feedbackResponseComment.getShowGiverNameTo().contains(FeedbackParticipantType.INSTRUCTORS); + return isCommentVisibleToInstructor && isGiverVisibleToInstructor; + } + } diff --git a/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java b/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java index a66593a7107..8e51742af89 100644 --- a/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java +++ b/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java @@ -2,8 +2,6 @@ import java.util.UUID; -import javax.ws.rs.HEAD; - import teammates.common.datatransfer.FeedbackResultFetchType; import teammates.common.datatransfer.SessionResultsBundle; import teammates.common.datatransfer.SqlSessionResultsBundle; @@ -199,7 +197,7 @@ private JsonResult executeWithSql( instructor = getPossiblyUnregisteredSqlInstructor(courseId); bundle = sqlLogic.getSessionResultsForUser(feedbackSession, courseId, instructor.getEmail(), - true, questionUuid); + true, questionUuid, isPreviewResults); // Build a fake student object, as the results will be displayed as if they are displayed to a student student = new Student(instructor.getCourse(), instructor.getName(), instructor.getEmail(), ""); @@ -211,7 +209,7 @@ private JsonResult executeWithSql( student = getSqlStudentOfCourseFromRequest(courseId); bundle = sqlLogic.getSessionResultsForUser(feedbackSession, courseId, student.getEmail(), - false, questionUuid); + false, questionUuid, isPreviewResults); return new JsonResult(SessionResultsData.initForStudent(bundle, student)); case INSTRUCTOR_SUBMISSION: diff --git a/src/test/java/teammates/common/datatransfer/SqlSessionResultsBundleTest.java b/src/test/java/teammates/common/datatransfer/SqlSessionResultsBundleTest.java index af600b9fd76..6661455b499 100644 --- a/src/test/java/teammates/common/datatransfer/SqlSessionResultsBundleTest.java +++ b/src/test/java/teammates/common/datatransfer/SqlSessionResultsBundleTest.java @@ -2,6 +2,7 @@ import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; @@ -30,6 +31,8 @@ public void testGetQuestionResponseMap() { SqlSessionResultsBundle bundle = new SqlSessionResultsBundle( new ArrayList<>(responseBundle.feedbackQuestions.values()), + new HashSet<>(), + new HashSet<>(), new ArrayList<>(responseBundle.feedbackResponses.values()), new ArrayList<>(), new HashMap<>(), @@ -66,6 +69,8 @@ public void testGetQuestionMissingResponseMap() { SqlSessionResultsBundle bundle = new SqlSessionResultsBundle( new ArrayList<>(responseBundle.feedbackQuestions.values()), + new HashSet<>(), + new HashSet<>(), new ArrayList<>(), new ArrayList<>(responseBundle.feedbackResponses.values()), new HashMap<>(), @@ -121,6 +126,8 @@ public void testIsResponseGiverRecipientVisible_typicalCase_shouldReturnCorrectV SqlSessionResultsBundle bundle = new SqlSessionResultsBundle( new ArrayList<>(responseBundle.feedbackQuestions.values()), + new HashSet<>(), + new HashSet<>(), new ArrayList<>(responseBundle.feedbackResponses.values()), new ArrayList<>(), responseGiverVisibilityTable, @@ -153,6 +160,8 @@ public void testIsCommentGiverVisible_typicalCase_shouldReturnCorrectValues() { SqlSessionResultsBundle bundle = new SqlSessionResultsBundle( new ArrayList<>(responseBundle.feedbackQuestions.values()), + new HashSet<>(), + new HashSet<>(), new ArrayList<>(responseBundle.feedbackResponses.values()), new ArrayList<>(), new HashMap<>(), From 2504ead81c14aa68695251f2fe6fffb4d73d5ff5 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Sat, 24 Feb 2024 12:44:40 +0800 Subject: [PATCH 32/42] Migrate preview functionality for question output --- .../teammates/ui/output/SessionResultsData.java | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/main/java/teammates/ui/output/SessionResultsData.java b/src/main/java/teammates/ui/output/SessionResultsData.java index d845c8d2b2a..1ea9568c8c6 100644 --- a/src/main/java/teammates/ui/output/SessionResultsData.java +++ b/src/main/java/teammates/ui/output/SessionResultsData.java @@ -84,7 +84,7 @@ public static SessionResultsData initForInstructor(SqlSessionResultsBundle bundl questionsWithResponses.forEach((question, responses) -> { FeedbackQuestionDetails questionDetails = question.getQuestionDetailsCopy(); QuestionOutput qnOutput = new QuestionOutput(question, - questionDetails.getQuestionResultStatisticsJson(question, null, bundle)); + questionDetails.getQuestionResultStatisticsJson(question, null, bundle), false, false); // put normal responses List allResponses = buildResponsesForInstructor(responses, bundle, false); qnOutput.allResponses.addAll(allResponses); @@ -176,8 +176,12 @@ public static SessionResultsData initForStudent(SqlSessionResultsBundle bundle, questionsWithResponses.forEach((question, responses) -> { FeedbackQuestionDetails questionDetails = question.getQuestionDetailsCopy(); + // check if question has comments (on any responses) not visible for preview + boolean hasCommentNotVisibleForPreview = bundle.getQuestionsWithCommentNotVisibleForPreviewSet() + .contains(question); QuestionOutput qnOutput = new QuestionOutput(question, - questionDetails.getQuestionResultStatisticsJson(question, student.getEmail(), bundle)); + questionDetails.getQuestionResultStatisticsJson(question, student.getEmail(), bundle), + false, hasCommentNotVisibleForPreview); Map> otherResponsesMap = new HashMap<>(); qnOutput.getFeedbackQuestion().hideInformationForStudent(); @@ -728,9 +732,12 @@ private QuestionOutput(FeedbackQuestionAttributes feedbackQuestionAttributes, St this.hasCommentNotVisibleForPreview = hasCommentNotVisibleForPreview; } - private QuestionOutput(FeedbackQuestion feedbackQuestion, String questionStatistics) { + private QuestionOutput(FeedbackQuestion feedbackQuestion, String questionStatistics, + boolean hasResponseButNotVisibleForPreview, boolean hasCommentNotVisibleForPreview) { this.feedbackQuestion = new FeedbackQuestionData(feedbackQuestion); this.questionStatistics = questionStatistics; + this.hasResponseButNotVisibleForPreview = hasResponseButNotVisibleForPreview; + this.hasCommentNotVisibleForPreview = hasCommentNotVisibleForPreview; } public FeedbackQuestionData getFeedbackQuestion() { From 381115447ea516b1d8d306a4ac81fc22ef4f99b1 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Mon, 26 Feb 2024 18:47:04 +0800 Subject: [PATCH 33/42] Fix recipient section filter --- .../java/teammates/storage/sqlapi/FeedbackResponsesDb.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java b/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java index c2c4ae70a26..28ae2b42b04 100644 --- a/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java +++ b/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java @@ -295,7 +295,7 @@ public List getFeedbackResponsesForSessionInSection( giverSectionFilter = cb.equal(giverJoin.get("name"), sectionName); } if (fetchType.shouldFetchByReceiver()) { - giverSectionFilter = cb.equal(recipientJoin.get("name"), sectionName); + recipientSectionFilter = cb.equal(recipientJoin.get("name"), sectionName); } cq.select(root) @@ -333,7 +333,7 @@ public List getFeedbackResponsesForQuestionInSection( giverSectionFilter = cb.equal(giverJoin.get("name"), sectionName); } if (fetchType.shouldFetchByReceiver()) { - giverSectionFilter = cb.equal(recipientJoin.get("name"), sectionName); + recipientSectionFilter = cb.equal(recipientJoin.get("name"), sectionName); } cq.select(root) From f8c18b888554505d31d52addb76b65024ba4605f Mon Sep 17 00:00:00 2001 From: Xenos F Date: Mon, 26 Feb 2024 19:21:43 +0800 Subject: [PATCH 34/42] Update test cases to handle question preview --- .../ui/webapi/GetSessionResultsActionIT.java | 37 ++++++++++++++++--- 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/src/it/java/teammates/it/ui/webapi/GetSessionResultsActionIT.java b/src/it/java/teammates/it/ui/webapi/GetSessionResultsActionIT.java index 89dc0703901..9d727b1c356 100644 --- a/src/it/java/teammates/it/ui/webapi/GetSessionResultsActionIT.java +++ b/src/it/java/teammates/it/ui/webapi/GetSessionResultsActionIT.java @@ -107,9 +107,32 @@ protected void testExecute() { } } - ______TS("Typical: Student accesses results of their course"); + ______TS("Typical: Instructor previews session results as student"); Student student = typicalBundle.students.get("student1InCourse1"); + + submissionParams = new String[] { + Const.ParamsNames.FEEDBACK_SESSION_NAME, accessibleFeedbackSession.getName(), + Const.ParamsNames.COURSE_ID, accessibleFeedbackSession.getCourse().getId(), + Const.ParamsNames.INTENT, Intent.STUDENT_RESULT.name(), + Const.ParamsNames.PREVIEWAS, student.getEmail(), + }; + + a = getAction(submissionParams); + r = getJsonResult(a); + + output = (SessionResultsData) r.getOutput(); + expectedResults = SessionResultsData.initForStudent( + logic.getSessionResultsForUser(accessibleFeedbackSession, + accessibleFeedbackSession.getCourse().getId(), + student.getEmail(), + false, null, true), + student); + + assertTrue(isSessionResultsDataEqual(expectedResults, output)); + + ______TS("Typical: Student accesses results of their course"); + loginAsStudent(student.getGoogleId()); submissionParams = new String[] { @@ -126,7 +149,7 @@ protected void testExecute() { logic.getSessionResultsForUser(accessibleFeedbackSession, accessibleFeedbackSession.getCourse().getId(), student.getEmail(), - false, null), + false, null, false), student); assertTrue(isSessionResultsDataEqual(expectedResults, output)); @@ -209,8 +232,10 @@ private boolean isSessionResultsDataEqual(SessionResultsData self, SessionResult private boolean isQuestionOutputEqual(SessionResultsData.QuestionOutput self, SessionResultsData.QuestionOutput other) { - if (!JsonUtils.toJson(self.getFeedbackQuestion()).equals(JsonUtils.toJson(self.getFeedbackQuestion())) - || !self.getQuestionStatistics().equals(other.getQuestionStatistics())) { + if (!JsonUtils.toJson(self.getFeedbackQuestion()).equals(JsonUtils.toJson(other.getFeedbackQuestion())) + || !self.getQuestionStatistics().equals(other.getQuestionStatistics()) + || self.getHasResponseButNotVisibleForPreview() != other.getHasResponseButNotVisibleForPreview() + || self.getHasCommentNotVisibleForPreview() != other.getHasCommentNotVisibleForPreview()) { return false; } List thisResponses; @@ -290,11 +315,11 @@ public void testAccessControl_studentAccessOwnCourseSessionResult_shouldPass() t public void testAccessControl_studentAccessUnpublishedSessionStudentResult_shouldFail() { Student student1InCourse1 = typicalBundle.students.get("student1InCourse1"); Course typicalCourse = typicalBundle.courses.get("course1"); - FeedbackSession feedbackSession = typicalBundle.feedbackSessions.get("session2InTypicalCourse"); + FeedbackSession unpublishedFeedbackSession = typicalBundle.feedbackSessions.get("session2InTypicalCourse"); String[] submissionParams = new String[] { Const.ParamsNames.COURSE_ID, typicalCourse.getId(), - Const.ParamsNames.FEEDBACK_SESSION_NAME, feedbackSession.getName(), + Const.ParamsNames.FEEDBACK_SESSION_NAME, unpublishedFeedbackSession.getName(), Const.ParamsNames.INTENT, Intent.STUDENT_RESULT.toString(), }; loginAsStudent(student1InCourse1.getGoogleId()); From 4a75f0e055b6de3f52b3005903cadf1750a06d31 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Mon, 26 Feb 2024 19:29:11 +0800 Subject: [PATCH 35/42] Merge duplicate methods --- .../sqllogic/core/FeedbackResponsesLogic.java | 9 +------- .../storage/sqlapi/FeedbackResponsesDb.java | 21 +------------------ 2 files changed, 2 insertions(+), 28 deletions(-) diff --git a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java index 997934b59f5..ca56d0d6cc6 100644 --- a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java +++ b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java @@ -312,7 +312,7 @@ public List getFeedbackResponsesFromGiverForQuestion( } /** - * Gets all responses given by a user for a question. + * Gets all responses for a question. */ public List getFeedbackResponsesForQuestion(UUID feedbackQuestionId) { return frDb.getResponsesForQuestion(feedbackQuestionId); @@ -1026,13 +1026,6 @@ public List getFeedbackResponsesForSessionInSection( return frDb.getFeedbackResponsesForSessionInSection(feedbackSession, courseId, sectionName, fetchType); } - /** - * Gets all responses for a question. - */ - public List getFeedbackResponsesForQuestion(UUID feedbackQuestionId) { - return frDb.getFeedbackResponsesForQuestion(feedbackQuestionId); - } - /** * Gets all responses given to/from a section for a question. * diff --git a/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java b/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java index 0f944bb57ee..7c79986719f 100644 --- a/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java +++ b/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java @@ -168,7 +168,7 @@ public boolean areThereResponsesForQuestion(UUID questionId) { } /** - * Get responses responses for a question. + * Get responses for a question. */ public List getResponsesForQuestion(UUID questionId) { CriteriaBuilder cb = HibernateUtil.getCriteriaBuilder(); @@ -262,25 +262,6 @@ public List getFeedbackResponsesForRecipientForQuestion( return HibernateUtil.createQuery(cq).getResultList(); } - /** - * Gets all feedback responses for a question. - */ - public List getFeedbackResponsesForQuestion(UUID questionId) { - assert questionId != null; - - CriteriaBuilder cb = HibernateUtil.getCriteriaBuilder(); - CriteriaQuery cq = cb.createQuery(FeedbackResponse.class); - Root root = cq.from(FeedbackResponse.class); - Join fqJoin = root.join("feedbackQuestion"); - - cq.select(root) - .where(cb.and( - cb.equal(fqJoin.get("id"), questionId) - )); - - return HibernateUtil.createQuery(cq).getResultList(); - } - /** * Gets all responses given to/from a section in a feedback session in a course. * Optionally, retrieves by either giver, receiver sections, or both. From 27ca58d98f98f5c7f9932db902c8dd38a20e43aa Mon Sep 17 00:00:00 2001 From: Xenos F Date: Mon, 26 Feb 2024 19:32:48 +0800 Subject: [PATCH 36/42] Fix checkstyle errors --- .../webapi/BasicFeedbackSubmissionAction.java | 38 +++++++++---------- .../ui/webapi/GetSessionResultsAction.java | 14 ++++--- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/src/main/java/teammates/ui/webapi/BasicFeedbackSubmissionAction.java b/src/main/java/teammates/ui/webapi/BasicFeedbackSubmissionAction.java index a4a45ecf00a..84ceb44b90d 100644 --- a/src/main/java/teammates/ui/webapi/BasicFeedbackSubmissionAction.java +++ b/src/main/java/teammates/ui/webapi/BasicFeedbackSubmissionAction.java @@ -129,25 +129,6 @@ void checkAccessControlForStudentFeedbackSubmission( } } - /** - * Checks the access control for student feedback result. - */ - void checkAccessControlForStudentFeedbackResult( - Student student, FeedbackSession feedbackSession) throws UnauthorizedAccessException { - if (student == null) { - throw new UnauthorizedAccessException("Trying to access system using a non-existent student entity"); - } - - String previewAsPerson = getRequestParamValue(Const.ParamsNames.PREVIEWAS); - - if (StringHelper.isEmpty(previewAsPerson)) { - gateKeeper.verifyAccessible(student, feedbackSession); - verifyMatchingGoogleId(student.getGoogleId()); - } else { - checkAccessControlForPreview(feedbackSession, false); - } - } - /** * Checks the access control for student feedback submission. */ @@ -204,6 +185,25 @@ void checkAccessControlForStudentFeedbackResult( } } + /** + * Checks the access control for student feedback result. + */ + void checkAccessControlForStudentFeedbackResult( + Student student, FeedbackSession feedbackSession) throws UnauthorizedAccessException { + if (student == null) { + throw new UnauthorizedAccessException("Trying to access system using a non-existent student entity"); + } + + String previewAsPerson = getRequestParamValue(Const.ParamsNames.PREVIEWAS); + + if (StringHelper.isEmpty(previewAsPerson)) { + gateKeeper.verifyAccessible(student, feedbackSession); + verifyMatchingGoogleId(student.getGoogleId()); + } else { + checkAccessControlForPreview(feedbackSession, false); + } + } + /** * Gets the instructor involved in the submission process. */ diff --git a/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java b/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java index 8e51742af89..04b2d1240fd 100644 --- a/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java +++ b/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java @@ -9,10 +9,10 @@ import teammates.common.datatransfer.attributes.InstructorAttributes; import teammates.common.datatransfer.attributes.StudentAttributes; import teammates.common.util.Const; +import teammates.common.util.StringHelper; import teammates.storage.sqlentity.FeedbackSession; import teammates.storage.sqlentity.Instructor; import teammates.storage.sqlentity.Student; -import teammates.common.util.StringHelper; import teammates.ui.output.SessionResultsData; import teammates.ui.request.Intent; @@ -43,7 +43,8 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException { } private void checkSpecificAccessControlDatastore( - String courseId, String feedbackSessionName, Intent intent, boolean isPreviewResults) throws UnauthorizedAccessException { + String courseId, String feedbackSessionName, Intent intent, boolean isPreviewResults) + throws UnauthorizedAccessException { FeedbackSessionAttributes feedbackSession = getNonNullFeedbackSession(feedbackSessionName, courseId); switch (intent) { case FULL_DETAIL: @@ -76,7 +77,8 @@ private void checkSpecificAccessControlDatastore( } private void checkSpecificAccessControlSql( - String courseId, String feedbackSessionName, Intent intent, boolean isPreviewResults) throws UnauthorizedAccessException { + String courseId, String feedbackSessionName, Intent intent, boolean isPreviewResults) + throws UnauthorizedAccessException { FeedbackSession feedbackSession = getNonNullSqlFeedbackSession(feedbackSessionName, courseId); switch (intent) { @@ -130,9 +132,11 @@ public JsonResult execute() { executeWithSql(courseId, feedbackSessionName, questionUuid, selectedSection, fetchType, intent, isPreviewResults); } - return executeWithSql(courseId, feedbackSessionName, null, selectedSection, fetchType, intent, isPreviewResults); + return executeWithSql(courseId, feedbackSessionName, null, selectedSection, + fetchType, intent, isPreviewResults); } else { - return executeWithDatastore(courseId, feedbackSessionName, questionId, selectedSection, fetchType, intent, isPreviewResults); + return executeWithDatastore(courseId, feedbackSessionName, questionId, selectedSection, + fetchType, intent, isPreviewResults); } } From 79fae7832423997d7bf83cc49f33fc06fbfce94e Mon Sep 17 00:00:00 2001 From: Xenos F Date: Mon, 26 Feb 2024 19:50:47 +0800 Subject: [PATCH 37/42] Add missing questions with non-visible preview responses --- src/main/java/teammates/ui/output/SessionResultsData.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/main/java/teammates/ui/output/SessionResultsData.java b/src/main/java/teammates/ui/output/SessionResultsData.java index 1ea9568c8c6..e8b88d9d0eb 100644 --- a/src/main/java/teammates/ui/output/SessionResultsData.java +++ b/src/main/java/teammates/ui/output/SessionResultsData.java @@ -7,6 +7,7 @@ import java.util.List; import java.util.Map; import java.util.Queue; +import java.util.Set; import javax.annotation.Nullable; @@ -221,6 +222,13 @@ public static SessionResultsData initForStudent(SqlSessionResultsBundle bundle, sessionResultsData.questions.add(qnOutput); }); + Set questionsWithResponsesNotVisibleForPreview = + bundle.getQuestionsNotVisibleForPreviewSet(); + questionsWithResponsesNotVisibleForPreview.forEach((question) -> { + QuestionOutput qnOutput = new QuestionOutput(question, "", true, false); + sessionResultsData.questions.add(qnOutput); + }); + return sessionResultsData; } From 5d8bf9501eca676835a3a7d3ddc763cf4354e704 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Mon, 26 Feb 2024 21:09:22 +0800 Subject: [PATCH 38/42] Remove outdated test --- .../it/ui/webapi/GetSessionResultsActionIT.java | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/src/it/java/teammates/it/ui/webapi/GetSessionResultsActionIT.java b/src/it/java/teammates/it/ui/webapi/GetSessionResultsActionIT.java index 9d727b1c356..246ea81b5a4 100644 --- a/src/it/java/teammates/it/ui/webapi/GetSessionResultsActionIT.java +++ b/src/it/java/teammates/it/ui/webapi/GetSessionResultsActionIT.java @@ -264,23 +264,6 @@ private boolean isResponseOutputEqual(SessionResultsData.ResponseOutput self, && self.getResponseDetails().getJsonString().equals(other.getResponseDetails().getJsonString()); } - @Test - public void testAccessControl_withRegistrationKey_shouldPass() throws Exception { - Course typicalCourse1 = typicalBundle.courses.get("course1"); - FeedbackSession feedbackSession = typicalBundle.feedbackSessions.get("session1InCourse1"); - Student student1 = typicalBundle.students.get("student1InCourse1"); - student1 = logic.getStudentForEmail(student1.getCourse().getId(), student1.getEmail()); - - String[] submissionParams = new String[] { - Const.ParamsNames.COURSE_ID, typicalCourse1.getId(), - Const.ParamsNames.FEEDBACK_SESSION_NAME, feedbackSession.getName(), - Const.ParamsNames.INTENT, Intent.STUDENT_RESULT.toString(), - Const.ParamsNames.REGKEY, student1.getRegKey(), - }; - - verifyAccessibleForUnregisteredUsers(submissionParams); - } - @Test public void testAccessControl_withoutCorrectAuthInfoAccessStudentResult_shouldFail() throws Exception { Course typicalCourse1 = typicalBundle.courses.get("course1"); From e10e4bab775ea30822e3c5b936f83689a40e803e Mon Sep 17 00:00:00 2001 From: Xenos F Date: Mon, 26 Feb 2024 21:26:40 +0800 Subject: [PATCH 39/42] Edit for style and readability --- .../teammates/sqllogic/core/FeedbackResponsesLogic.java | 7 ++++--- src/main/java/teammates/ui/output/SessionResultsData.java | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java index ca56d0d6cc6..aef4f49967d 100644 --- a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java +++ b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java @@ -36,7 +36,7 @@ import teammates.storage.sqlentity.responses.FeedbackRankRecipientsResponse; /** - * Handles operations related to feedback sessions. + * Handles operations related to feedback responses. * * @see FeedbackResponse * @see FeedbackResponsesDb @@ -581,6 +581,8 @@ private SqlSessionResultsBundle buildResultsBundle( List relatedQuestions = new ArrayList<>(); List relatedResponses = new ArrayList<>(); Map> relatedCommentsMap = new HashMap<>(); + Set relatedQuestionsNotVisibleForPreviewSet = new HashSet<>(); + Set relatedQuestionsWithCommentNotVisibleForPreview = new HashSet<>(); if (isCourseWide) { // all questions are related questions when viewing course-wide result for (FeedbackQuestion qn : allQuestions) { @@ -600,8 +602,6 @@ private SqlSessionResultsBundle buildResultsBundle( Map responseGiverVisibilityTable = new HashMap<>(); Map responseRecipientVisibilityTable = new HashMap<>(); Map commentVisibilityTable = new HashMap<>(); - Set relatedQuestionsNotVisibleForPreviewSet = new HashSet<>(); - Set relatedQuestionsWithCommentNotVisibleForPreview = new HashSet<>(); // build response for (FeedbackResponse response : allResponses) { @@ -736,6 +736,7 @@ public SqlSessionResultsBundle getSessionResultsForCourse( * @param userEmail the user viewing the feedback session * @param isInstructor true if the user is an instructor * @param questionId if not null, will only return partial bundle for the question + * @param isPreviewResults true if getting session results for preview purpose * @return the session result bundle */ public SqlSessionResultsBundle getSessionResultsForUser( diff --git a/src/main/java/teammates/ui/output/SessionResultsData.java b/src/main/java/teammates/ui/output/SessionResultsData.java index e8b88d9d0eb..796aba29558 100644 --- a/src/main/java/teammates/ui/output/SessionResultsData.java +++ b/src/main/java/teammates/ui/output/SessionResultsData.java @@ -224,7 +224,7 @@ public static SessionResultsData initForStudent(SqlSessionResultsBundle bundle, Set questionsWithResponsesNotVisibleForPreview = bundle.getQuestionsNotVisibleForPreviewSet(); - questionsWithResponsesNotVisibleForPreview.forEach((question) -> { + questionsWithResponsesNotVisibleForPreview.forEach(question -> { QuestionOutput qnOutput = new QuestionOutput(question, "", true, false); sessionResultsData.questions.add(qnOutput); }); From 343bd0e210cbd2fcb4ed6b99693d8fa2021e6cde Mon Sep 17 00:00:00 2001 From: Xenos F Date: Tue, 27 Feb 2024 00:52:55 +0800 Subject: [PATCH 40/42] Fix missing join --- .../teammates/storage/sqlapi/FeedbackResponseCommentsDb.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java b/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java index 8a0b781eaeb..6b43ca4e96c 100644 --- a/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java +++ b/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java @@ -292,7 +292,8 @@ public List getFeedbackResponseCommentsForQuestionInSec CriteriaBuilder cb = HibernateUtil.getCriteriaBuilder(); CriteriaQuery cq = cb.createQuery(FeedbackResponseComment.class); Root root = cq.from(FeedbackResponseComment.class); - Join fqJoin = root.join("feedbackQuestion"); + Join frJoin = root.join("feedbackResponse"); + Join fqJoin = frJoin.join("feedbackQuestion"); Join fsJoin = fqJoin.join("feedbackSession"); Join cJoin = fsJoin.join("course"); ListJoin sectionsJoin = cJoin.joinList("sections"); From 36eb927174f96fac1879ad4b5c13a7f0c84e9260 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Tue, 27 Feb 2024 01:23:56 +0800 Subject: [PATCH 41/42] Fix section filtering logic --- .../sqlapi/FeedbackResponseCommentsDb.java | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java b/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java index 6b43ca4e96c..9a8b7c60e8c 100644 --- a/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java +++ b/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java @@ -254,7 +254,7 @@ public List getFeedbackResponseCommentsForQuestion(UUID } /** - * Gets all comments which have its corresponding response given to/from a section of a feedback session of a course. + * Gets all comments in the given session where the giver or recipient is in the given section. */ public List getFeedbackResponseCommentsForSessionInSection( String courseId, String feedbackSessionName, String sectionName) { @@ -269,20 +269,24 @@ public List getFeedbackResponseCommentsForSessionInSect Join fqJoin = frJoin.join("feedbackQuestion"); Join fsJoin = fqJoin.join("feedbackSession"); Join cJoin = fsJoin.join("course"); - ListJoin sectionsJoin = cJoin.joinList("sections"); + + Join giverJoin = root.join("giverSection"); + Join recipientJoin = root.join("recipientSection"); cq.select(root) .where(cb.and( cb.equal(cJoin.get("id"), courseId), cb.equal(fsJoin.get("name"), feedbackSessionName), - cb.in(cb.literal(sectionName)).value(sectionsJoin.get("name")) + cb.or( + cb.equal(giverJoin.get("name"), sectionName), + cb.equal(recipientJoin.get("name"), sectionName)) )); return HibernateUtil.createQuery(cq).getResultList(); } /** - * Gets all comments which have its corresponding response given to/from a section of a feedback question of a course. + * Gets all comments for a question where the giver or recipient is in the given section. */ public List getFeedbackResponseCommentsForQuestionInSection( UUID questionId, String sectionName) { @@ -294,14 +298,16 @@ public List getFeedbackResponseCommentsForQuestionInSec Root root = cq.from(FeedbackResponseComment.class); Join frJoin = root.join("feedbackResponse"); Join fqJoin = frJoin.join("feedbackQuestion"); - Join fsJoin = fqJoin.join("feedbackSession"); - Join cJoin = fsJoin.join("course"); - ListJoin sectionsJoin = cJoin.joinList("sections"); + + Join giverJoin = root.join("giverSection"); + Join recipientJoin = root.join("recipientSection"); cq.select(root) .where(cb.and( cb.equal(fqJoin.get("id"), questionId), - cb.in(cb.literal(sectionName)).value(sectionsJoin.get("name")) + cb.or( + cb.equal(giverJoin.get("name"), sectionName), + cb.equal(recipientJoin.get("name"), sectionName)) )); return HibernateUtil.createQuery(cq).getResultList(); From f762d0e1265afa26cc5712c6479d23dcd9edb1d2 Mon Sep 17 00:00:00 2001 From: Xenos F Date: Tue, 27 Feb 2024 01:33:49 +0800 Subject: [PATCH 42/42] Fix checkstyle errors --- .../teammates/storage/sqlapi/FeedbackResponseCommentsDb.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java b/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java index 9a8b7c60e8c..cd0f59c58c8 100644 --- a/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java +++ b/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java @@ -20,7 +20,6 @@ import jakarta.persistence.criteria.CriteriaBuilder; import jakarta.persistence.criteria.CriteriaQuery; import jakarta.persistence.criteria.Join; -import jakarta.persistence.criteria.ListJoin; import jakarta.persistence.criteria.Root; /** @@ -298,7 +297,7 @@ public List getFeedbackResponseCommentsForQuestionInSec Root root = cq.from(FeedbackResponseComment.class); Join frJoin = root.join("feedbackResponse"); Join fqJoin = frJoin.join("feedbackQuestion"); - + Join giverJoin = root.join("giverSection"); Join recipientJoin = root.join("recipientSection");