From c4228c7a7c49ed0e5d30f8e8a1c0d3a37f290fb8 Mon Sep 17 00:00:00 2001 From: Fergus Mok Date: Wed, 31 Jan 2024 22:48:37 +0800 Subject: [PATCH] Initial draft for migrated Remove unnecessary comments Remove unnecessary comments Save progress Add draft Add draft --- .../storage/sqlapi/FeedbackResponsesDbIT.java | 14 + .../java/teammates/common/util/Const.java | 3 + .../java/teammates/sqllogic/api/Logic.java | 52 ++++ .../core/FeedbackResponseCommentsLogic.java | 20 ++ .../sqllogic/core/FeedbackResponsesLogic.java | 56 ++++ .../teammates/storage/sqlapi/EntitiesDb.java | 15 + .../sqlapi/FeedbackResponseCommentsDb.java | 76 ++++- .../storage/sqlapi/FeedbackResponsesDb.java | 28 ++ .../storage/sqlentity/FeedbackQuestion.java | 4 + .../storage/sqlentity/FeedbackResponse.java | 27 +- .../sqlentity/FeedbackResponseComment.java | 7 +- .../ui/output/FeedbackResponsesData.java | 14 +- .../webapi/BasicFeedbackSubmissionAction.java | 17 +- .../webapi/SubmitFeedbackResponsesAction.java | 265 ++++++++++++++++-- 14 files changed, 567 insertions(+), 31 deletions(-) diff --git a/src/it/java/teammates/it/storage/sqlapi/FeedbackResponsesDbIT.java b/src/it/java/teammates/it/storage/sqlapi/FeedbackResponsesDbIT.java index 40b825ea016..990ebdb1044 100644 --- a/src/it/java/teammates/it/storage/sqlapi/FeedbackResponsesDbIT.java +++ b/src/it/java/teammates/it/storage/sqlapi/FeedbackResponsesDbIT.java @@ -73,6 +73,20 @@ public void testDeleteFeedbackResponsesForQuestionCascade() { assertNull(frcDb.getFeedbackResponseComment(frc1.getId())); } + @Test + public void testDeleteFeedbackResponsesAndCommentsCascade() { + ______TS("success: typical case"); + FeedbackResponse fr1 = typicalDataBundle.feedbackResponses.get("response1ForQ1"); + FeedbackResponseComment frc1 = typicalDataBundle.feedbackResponseComments.get("comment1ToResponse1ForQ1"); + + // List actualFrc = frcDb.getFeedbackResponseCommentForResponse(fr1.getId()); + frDb.deleteFeedbackResponsesAndCommentsCascade(fr1); + + assertNull(frDb.getFeedbackResponse(fr1.getId())); + + assertNull(frcDb.getFeedbackResponseComment(frc1.getId())); + } + @Test public void testHasResponsesFromGiverInSession() { ______TS("success: typical case"); diff --git a/src/main/java/teammates/common/util/Const.java b/src/main/java/teammates/common/util/Const.java index 01839aa197f..b4c8c1cd259 100644 --- a/src/main/java/teammates/common/util/Const.java +++ b/src/main/java/teammates/common/util/Const.java @@ -5,6 +5,8 @@ import java.time.Duration; import java.time.Instant; +import teammates.storage.sqlentity.Section; + /** * Stores constants that are widely used across classes. * this class contains several nested classes, each containing a specific @@ -25,6 +27,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_SQL_SECTION = null; public static final String UNKNOWN_INSTITUTION = "Unknown Institution"; diff --git a/src/main/java/teammates/sqllogic/api/Logic.java b/src/main/java/teammates/sqllogic/api/Logic.java index 44a167b646c..a735563798e 100644 --- a/src/main/java/teammates/sqllogic/api/Logic.java +++ b/src/main/java/teammates/sqllogic/api/Logic.java @@ -8,6 +8,7 @@ import javax.annotation.Nullable; +import teammates.common.datatransfer.DataBundle; import teammates.common.datatransfer.FeedbackQuestionRecipient; import teammates.common.datatransfer.NotificationStyle; import teammates.common.datatransfer.NotificationTargetUser; @@ -1147,6 +1148,57 @@ public FeedbackResponse getFeedbackResponse(UUID frId) { return feedbackResponsesLogic.getFeedbackResponse(frId); } + /** + * Creates a feedback response. + * + *
Preconditions:
+ * * All parameters are non-null. + * + * @return created feedback response + * @throws InvalidParametersException if the response is not valid + * @throws EntityAlreadyExistsException if the response already exist + */ + public FeedbackResponse createFeedbackResponse(FeedbackResponse feedbackResponse) + throws InvalidParametersException, EntityAlreadyExistsException { + assert feedbackResponse != null; + return feedbackResponsesLogic.createFeedbackResponse(feedbackResponse); + } + + /** + * Updates a non-null feedback response. + * + *

Cascade updates its associated feedback response comment + * (e.g. associated response ID, giverSection and recipientSection). + * + *

If the giver/recipient field is changed, the response is updated by recreating the response + * as question-giver-recipient is the primary key. + * + *
Preconditions:
+ * * All parameters are non-null. + * + * @return updated feedback response + * @throws InvalidParametersException if attributes to update are not valid + * @throws EntityDoesNotExistException if the comment cannot be found + * @throws EntityAlreadyExistsException if the response cannot be updated + * by recreation because of an existent response + */ + public FeedbackResponse updateFeedbackResponseCascade(FeedbackResponse feedbackResponse) + throws InvalidParametersException, EntityDoesNotExistException, EntityAlreadyExistsException { + assert feedbackResponse != null; + return feedbackResponsesLogic.updateFeedbackResponseCascade(feedbackResponse); + } + + /** + * Deletes a feedback response cascade its associated comments. + * + *
Preconditions:
+ * * All parameters are non-null. + */ + public void deleteFeedbackResponsesAndCommentsCascade(FeedbackResponse feedbackResponse) { + assert feedbackResponse != null; + feedbackResponsesLogic.deleteFeedbackResponsesAndCommentsCascade(feedbackResponse.getId()); + } + /** * Get existing feedback responses from instructor for the given question. */ diff --git a/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java b/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java index c6aabfa0153..e5446d1de75 100644 --- a/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java +++ b/src/main/java/teammates/sqllogic/core/FeedbackResponseCommentsLogic.java @@ -65,6 +65,13 @@ public List getFeedbackResponseCommentsForResponse(UUID /** * Gets the comment associated with the response. */ + public List getFeedbackResponseCommentForResponse(UUID feedbackResponseId) { + return frcDb.getFeedbackResponseCommentForResponse(feedbackResponseId); + } + + /** TODO: If there's a bug here, then update the comment + * Gets the comment associated with the response. + */ public FeedbackResponseComment getFeedbackResponseCommentForResponseFromParticipant( UUID feedbackResponseId) { return frcDb.getFeedbackResponseCommentForResponseFromParticipant(feedbackResponseId); @@ -87,6 +94,19 @@ public void deleteFeedbackResponseComment(Long frcId) { frcDb.deleteFeedbackResponseComment(frcId); } + /** + * Updates a feedback response comment by {@link FeedbackResponseComment}. + * + * @return updated comment + * @throws InvalidParametersException if attributes to update are not valid + * @throws EntityDoesNotExistException if the comment cannot be found + */ + public FeedbackResponseComment updateFeedbackResponseComment(FeedbackResponseComment feedbackResponseComment) + throws InvalidParametersException, EntityDoesNotExistException { + + return frcDb.updateFeedbackResponseComment(feedbackResponseComment); + } + /** * Updates a feedback response comment. * @throws EntityDoesNotExistException if the comment does not exist diff --git a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java index 7e3cb8d13e4..edcb6408ed6 100644 --- a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java +++ b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java @@ -18,6 +18,7 @@ import teammates.storage.sqlentity.Course; 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; @@ -176,6 +177,61 @@ private List getFeedbackResponsesFromTeamForQuestion( return responses; } + /** + * Updates a non-null feedback response by {@link FeedbackResponse}. + * + *

Cascade updates its associated feedback response comment + * (e.g. associated response ID, giverSection and recipientSection). + * + *

If the giver/recipient field is changed, the response is updated by recreating the response + * as question-giver-recipient is the primary key. + * + * @return updated feedback response + * @throws InvalidParametersException if attributes to update are not valid + * @throws EntityDoesNotExistException if the comment cannot be found + * @throws EntityAlreadyExistsException if the response cannot be updated + * by recreation because of an existent response + */ + public FeedbackResponse updateFeedbackResponseCascade(FeedbackResponse feedbackResponse) + throws InvalidParametersException, EntityDoesNotExistException, EntityAlreadyExistsException { + + FeedbackResponse oldResponse = frDb.getFeedbackResponse(feedbackResponse.getId()); + FeedbackResponse newResponse = frDb.updateFeedbackResponse(feedbackResponse); + + boolean isResponseIdChanged = !oldResponse.getId().equals(newResponse.getId()); + boolean isGiverSectionChanged = !oldResponse.getGiverSection().equals(newResponse.getGiverSection()); + boolean isRecipientSectionChanged = !oldResponse.getRecipientSection().equals(newResponse.getRecipientSection()); + + if (isResponseIdChanged || isGiverSectionChanged || isRecipientSectionChanged) { + List oldResponseComments = + frcLogic.getFeedbackResponseCommentForResponse(oldResponse.getId()); + for (FeedbackResponseComment oldResponseComment : oldResponseComments) { + if (isResponseIdChanged) { + oldResponseComment.setFeedbackResponse(newResponse); + } + + if (isGiverSectionChanged) { + oldResponseComment.setGiverSection(newResponse.getGiverSection()); + } + + if (isRecipientSectionChanged) { + oldResponseComment.setRecipientSection(newResponse.getRecipientSection()); + } + + frcLogic.updateFeedbackResponseComment(oldResponseComment); + } + + } + return newResponse; + } + + /** + * Deletes a feedback response cascade its associated feedback response comments. + */ + public void deleteFeedbackResponsesAndCommentsCascade(UUID feedbackResponseId) { + frDb.deleteFeedbackResponsesAndCommentsCascade(feedbackResponseId); + } + /** * Deletes all feedback responses of a question cascade its associated comments. */ diff --git a/src/main/java/teammates/storage/sqlapi/EntitiesDb.java b/src/main/java/teammates/storage/sqlapi/EntitiesDb.java index 8ad58987ab2..a880bbfb533 100644 --- a/src/main/java/teammates/storage/sqlapi/EntitiesDb.java +++ b/src/main/java/teammates/storage/sqlapi/EntitiesDb.java @@ -1,6 +1,8 @@ package teammates.storage.sqlapi; +import com.google.common.base.Objects; + import teammates.common.util.HibernateUtil; import teammates.common.util.Logger; import teammates.storage.sqlentity.BaseEntity; @@ -10,6 +12,12 @@ */ class EntitiesDb { + /** + * Info message when entity is not saved because it does not change. + */ + static final String OPTIMIZED_SAVING_POLICY_APPLIED = + "Saving request is not issued because entity %s does not change by the update (%s)"; + static final Logger log = Logger.getLogger(); /** @@ -43,4 +51,11 @@ protected void delete(BaseEntity entity) { HibernateUtil.remove(entity); log.info("Entity deleted: " + entity.toString()); } + + /** + * Checks whether two values are the same. + */ + boolean hasSameValue(T oldValue, T newValue) { + return Objects.equal(oldValue, newValue); + } } diff --git a/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java b/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java index 9aff2b7251d..495f311ea8f 100644 --- a/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java +++ b/src/main/java/teammates/storage/sqlapi/FeedbackResponseCommentsDb.java @@ -1,11 +1,15 @@ package teammates.storage.sqlapi; import static teammates.common.util.Const.ERROR_CREATE_ENTITY_ALREADY_EXISTS; +import static teammates.common.util.Const.ERROR_UPDATE_NON_EXISTENT; +import java.time.Instant; import java.util.List; import java.util.UUID; +import teammates.common.datatransfer.FeedbackParticipantType; import teammates.common.exception.EntityAlreadyExistsException; +import teammates.common.exception.EntityDoesNotExistException; import teammates.common.exception.InvalidParametersException; import teammates.common.util.HibernateUtil; import teammates.storage.sqlentity.Course; @@ -13,11 +17,13 @@ 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.CriteriaDelete; 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. @@ -78,6 +84,24 @@ public void deleteFeedbackResponseComment(Long frcId) { } } + /** + * Deletes all feedbackResponseComments based on feedback response ID. + */ + public void deleteFeedbackResponseCommentForFeedbackResponseCascade(UUID feedbackResponseId) { + assert feedbackResponseId != null; + + CriteriaBuilder cb = HibernateUtil.getCriteriaBuilder(); + CriteriaDelete cd = cb.createCriteriaDelete(FeedbackResponseComment.class); + Root sRoot = cd.from(FeedbackResponseComment.class); + Subquery subquery = cd.subquery(UUID.class); + Root subqueryRoot = subquery.from(FeedbackResponseComment.class); + Join sqJoin = subqueryRoot.join("feedbackResponse"); + subquery.select(subqueryRoot.get("id")); + subquery.where(cb.equal(sqJoin.get("id"), feedbackResponseId)); + cd.where(cb.in(sRoot.get("id")).value(subquery)); + HibernateUtil.createMutationQuery(cd).executeUpdate(); + } + /** * Gets all feedback response comments for a response. */ @@ -110,6 +134,56 @@ public FeedbackResponseComment getFeedbackResponseCommentForResponseFromParticip return HibernateUtil.createQuery(cq).getResultStream().findFirst().orElse(null); } + /** + * Updates a feedback response comment by {@link FeedbackResponseComment}. + * + * @return updated feedback response comment + * @throws InvalidParametersException if attributes to update are not valid + * @throws EntityDoesNotExistException if the comment cannot be found + */ + public FeedbackResponseComment updateFeedbackResponseComment(FeedbackResponseComment newFeedbackResponseComment) + throws InvalidParametersException, EntityDoesNotExistException { + assert newFeedbackResponseComment != null; + + FeedbackResponseComment oldFeedbackResponseComment = getFeedbackResponseComment(newFeedbackResponseComment.getId()); + if (oldFeedbackResponseComment == null) { + throw new EntityDoesNotExistException(ERROR_UPDATE_NON_EXISTENT + newFeedbackResponseComment); + } + + newFeedbackResponseComment.sanitizeForSaving(); + if (!newFeedbackResponseComment.isValid()) { + throw new InvalidParametersException(newFeedbackResponseComment.getInvalidityInfo()); + } + + // update only if change + boolean hasSameAttributes = + this.hasSameValue(newFeedbackResponseComment.getId(), oldFeedbackResponseComment.getId()) + && this.hasSameValue( + newFeedbackResponseComment.getCommentText(), oldFeedbackResponseComment.getCommentText()) + && this.>hasSameValue( + newFeedbackResponseComment.getShowCommentTo(), oldFeedbackResponseComment.getShowCommentTo()) + && this.>hasSameValue( + newFeedbackResponseComment.getShowGiverNameTo(), oldFeedbackResponseComment.getShowGiverNameTo()) + && this.hasSameValue( + newFeedbackResponseComment.getLastEditorEmail(), oldFeedbackResponseComment.getLastEditorEmail()) + && this.hasSameValue( + newFeedbackResponseComment.getUpdatedAt(), oldFeedbackResponseComment.getUpdatedAt()) + && this.

hasSameValue( + newFeedbackResponseComment.getGiverSection(), oldFeedbackResponseComment.getGiverSection()) + && this.
hasSameValue( + newFeedbackResponseComment.getRecipientSection(), oldFeedbackResponseComment.getRecipientSection()); + if (hasSameAttributes) { + log.info(String.format( + OPTIMIZED_SAVING_POLICY_APPLIED, + FeedbackResponseComment.class.getSimpleName(), + newFeedbackResponseComment)); + return newFeedbackResponseComment; + } + + merge(newFeedbackResponseComment); + return newFeedbackResponseComment; + } + /** * Updates the giver email for all of the giver's comments in a course. */ diff --git a/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java b/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java index e5b26e93f0d..4bf5251bd3f 100644 --- a/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java +++ b/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java @@ -13,6 +13,7 @@ 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 jakarta.persistence.criteria.CriteriaBuilder; @@ -107,6 +108,28 @@ public FeedbackResponse createFeedbackResponse(FeedbackResponse feedbackResponse return feedbackResponse; } + /** + * Saves an updated {@code FeedbackResponse} to the db. + * + * @return updated feedback response + * @throws InvalidParametersException if attributes to update are not valid + * @throws EntityDoesNotExistException if the feedback response cannot be found + */ + public FeedbackResponse updateFeedbackResponse(FeedbackResponse feedbackResponse) + throws InvalidParametersException, EntityDoesNotExistException { + + if (!feedbackResponse.isValid()) { + throw new InvalidParametersException(feedbackResponse.getInvalidityInfo()); + } + + if (getFeedbackResponse(feedbackResponse.getId()) == null) { + throw new EntityDoesNotExistException(ERROR_UPDATE_NON_EXISTENT); + } + + return merge(feedbackResponse); + } + + /** * Deletes a feedbackResponse. */ @@ -134,6 +157,11 @@ public List getFeedbackResponsesFromGiverForQuestion( return HibernateUtil.createQuery(cq).getResultList(); } + /** + * Deletes a feed back response and all it's associated feedback response comments. + */ + public void deleteFeedbackResponsesAndCommentsCascade(UUID feedbackResponseId) { } + /** * Deletes all feedback responses of a question cascade its associated comments. */ diff --git a/src/main/java/teammates/storage/sqlentity/FeedbackQuestion.java b/src/main/java/teammates/storage/sqlentity/FeedbackQuestion.java index 19f81cf0678..f90cc71327b 100644 --- a/src/main/java/teammates/storage/sqlentity/FeedbackQuestion.java +++ b/src/main/java/teammates/storage/sqlentity/FeedbackQuestion.java @@ -243,6 +243,10 @@ public void setFeedbackSession(FeedbackSession feedbackSession) { this.feedbackSession = feedbackSession; } + public String getFeedbackSessionName() { + return feedbackSession.getName(); + } + public List getFeedbackResponses() { return feedbackResponses; } diff --git a/src/main/java/teammates/storage/sqlentity/FeedbackResponse.java b/src/main/java/teammates/storage/sqlentity/FeedbackResponse.java index 983e12126ba..153830d1dc1 100644 --- a/src/main/java/teammates/storage/sqlentity/FeedbackResponse.java +++ b/src/main/java/teammates/storage/sqlentity/FeedbackResponse.java @@ -6,6 +6,8 @@ import java.util.Objects; import java.util.UUID; +import org.hibernate.annotations.OnDelete; +import org.hibernate.annotations.OnDeleteAction; import org.hibernate.annotations.UpdateTimestamp; import teammates.common.datatransfer.questions.FeedbackResponseDetails; @@ -43,7 +45,7 @@ public abstract class FeedbackResponse extends BaseEntity { @JoinColumn(name = "questionId") private FeedbackQuestion feedbackQuestion; - @OneToMany(mappedBy = "feedbackResponse", cascade = CascadeType.REMOVE) + @OneToMany(mappedBy = "feedbackResponse", cascade = {CascadeType.REMOVE, CascadeType.PERSIST}) private List feedbackResponseComments = new ArrayList<>(); @Column(nullable = false) @@ -140,6 +142,29 @@ public static FeedbackResponse makeResponse( return feedbackResponse; } + /** + * Update a feedback response according to its {@code FeedbackQuestionType}. + */ + public static FeedbackResponse updateResponse( + FeedbackResponse originalFeedbackResponse, + FeedbackQuestion feedbackQuestion, String giver, + Section giverSection, String receiver, Section receiverSection, + FeedbackResponseDetails responseDetails + ) { + FeedbackResponse updatedFeedbackResponse = FeedbackResponse.makeResponse( + feedbackQuestion, + giver, + giverSection, + receiver, + receiverSection, + responseDetails + ); + updatedFeedbackResponse.setCreatedAt(originalFeedbackResponse.getCreatedAt()); + updatedFeedbackResponse.setId(originalFeedbackResponse.getId()); + return updatedFeedbackResponse; + } + + /** * Gets a copy of the question details of the feedback question. */ diff --git a/src/main/java/teammates/storage/sqlentity/FeedbackResponseComment.java b/src/main/java/teammates/storage/sqlentity/FeedbackResponseComment.java index 311c836ae8f..a0f7c750c50 100644 --- a/src/main/java/teammates/storage/sqlentity/FeedbackResponseComment.java +++ b/src/main/java/teammates/storage/sqlentity/FeedbackResponseComment.java @@ -11,7 +11,7 @@ import teammates.common.datatransfer.FeedbackParticipantType; import teammates.common.util.FieldValidator; - +import teammates.common.util.SanitizationHelper; import jakarta.persistence.Column; import jakarta.persistence.Convert; import jakarta.persistence.Entity; @@ -202,6 +202,11 @@ public void setLastEditorEmail(String lastEditorEmail) { this.lastEditorEmail = lastEditorEmail; } + // TODO: Override when BaseEntity adds abstract sanitizeForSaving + public void sanitizeForSaving() { + this.commentText = SanitizationHelper.sanitizeForRichText(this.commentText); + } + @Override public List getInvalidityInfo() { List errors = new ArrayList<>(); diff --git a/src/main/java/teammates/ui/output/FeedbackResponsesData.java b/src/main/java/teammates/ui/output/FeedbackResponsesData.java index 74db4703828..2147994613b 100644 --- a/src/main/java/teammates/ui/output/FeedbackResponsesData.java +++ b/src/main/java/teammates/ui/output/FeedbackResponsesData.java @@ -5,6 +5,7 @@ import java.util.stream.Collectors; import teammates.common.datatransfer.attributes.FeedbackResponseAttributes; +import teammates.storage.sqlentity.FeedbackResponse; /** * The API output format of a list of {@link FeedbackResponseAttributes}. @@ -13,8 +14,17 @@ public class FeedbackResponsesData extends ApiOutput { private List responses; - public FeedbackResponsesData(List responses) { - this.responses = responses.stream().map(FeedbackResponseData::new).collect(Collectors.toList()); + private FeedbackResponsesData(List responses) { + this.responses = responses; + } + + // TODO: When deleting attributes, make constructor to be createFromEntity + public static FeedbackResponsesData createFromAttributes(List responses) { + return new FeedbackResponsesData(responses.stream().map(FeedbackResponseData::new).collect(Collectors.toList())); + } + + public static FeedbackResponsesData createFromEntity(List responses) { + return new FeedbackResponsesData(responses.stream().map(FeedbackResponseData::new).collect(Collectors.toList())); } public FeedbackResponsesData() { diff --git a/src/main/java/teammates/ui/webapi/BasicFeedbackSubmissionAction.java b/src/main/java/teammates/ui/webapi/BasicFeedbackSubmissionAction.java index 12da4a22778..526dad47047 100644 --- a/src/main/java/teammates/ui/webapi/BasicFeedbackSubmissionAction.java +++ b/src/main/java/teammates/ui/webapi/BasicFeedbackSubmissionAction.java @@ -321,47 +321,44 @@ void verifySessionOpenExceptForModeration(FeedbackSession feedbackSession) throw /** * Gets the section of a recipient. */ - String getRecipientSection( + Section getRecipientSection( String courseId, FeedbackParticipantType giverType, FeedbackParticipantType recipientType, String recipientIdentifier) { - if (!isCourseMigrated(courseId)) { - return getDatastoreRecipientSection(courseId, giverType, recipientType, recipientIdentifier); - } switch (recipientType) { case SELF: switch (giverType) { case INSTRUCTORS: case SELF: - return Const.DEFAULT_SECTION; + return Const.DEFAULT_SQL_SECTION; case TEAMS: case TEAMS_IN_SAME_SECTION: Section section = sqlLogic.getSectionByCourseIdAndTeam(courseId, recipientIdentifier); - return section == null ? Const.DEFAULT_SECTION : section.getName(); + return section == null ? Const.DEFAULT_SQL_SECTION : section; case STUDENTS: case STUDENTS_IN_SAME_SECTION: Student student = sqlLogic.getStudentForEmail(courseId, recipientIdentifier); - return student == null ? Const.DEFAULT_SECTION : student.getSectionName(); + return student == null ? Const.DEFAULT_SQL_SECTION : student.getSection(); default: assert false : "Invalid giver type " + giverType + " for recipient type " + recipientType; return null; } case INSTRUCTORS: case NONE: - return Const.DEFAULT_SECTION; + return Const.DEFAULT_SQL_SECTION; case TEAMS: case TEAMS_EXCLUDING_SELF: case TEAMS_IN_SAME_SECTION: case OWN_TEAM: Section section = sqlLogic.getSectionByCourseIdAndTeam(courseId, recipientIdentifier); - return section == null ? Const.DEFAULT_SECTION : section.getName(); + return section == null ? Const.DEFAULT_SQL_SECTION : section; case STUDENTS: case STUDENTS_EXCLUDING_SELF: case STUDENTS_IN_SAME_SECTION: case OWN_TEAM_MEMBERS: case OWN_TEAM_MEMBERS_INCLUDING_SELF: Student student = sqlLogic.getStudentForEmail(courseId, recipientIdentifier); - return student == null ? Const.DEFAULT_SECTION : student.getTeamName(); + return student == null ? Const.DEFAULT_SQL_SECTION : student.getSection(); default: assert false : "Unknown recipient type " + recipientType; return null; diff --git a/src/main/java/teammates/ui/webapi/SubmitFeedbackResponsesAction.java b/src/main/java/teammates/ui/webapi/SubmitFeedbackResponsesAction.java index 35ca9bcb4ef..efe5e3c80e3 100644 --- a/src/main/java/teammates/ui/webapi/SubmitFeedbackResponsesAction.java +++ b/src/main/java/teammates/ui/webapi/SubmitFeedbackResponsesAction.java @@ -4,6 +4,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.UUID; import java.util.stream.Collectors; import teammates.common.datatransfer.FeedbackParticipantType; @@ -20,6 +21,12 @@ import teammates.common.util.Const; import teammates.common.util.JsonUtils; import teammates.common.util.Logger; +import teammates.storage.sqlentity.FeedbackQuestion; +import teammates.storage.sqlentity.FeedbackResponse; +import teammates.storage.sqlentity.FeedbackSession; +import teammates.storage.sqlentity.Instructor; +import teammates.storage.sqlentity.Section; +import teammates.storage.sqlentity.Student; import teammates.ui.output.FeedbackResponsesData; import teammates.ui.request.FeedbackResponsesRequest; import teammates.ui.request.Intent; @@ -43,13 +50,37 @@ AuthType getMinAuthLevel() { @Override void checkSpecificAccessControl() throws UnauthorizedAccessException { String feedbackQuestionId = getNonNullRequestParamValue(Const.ParamsNames.FEEDBACK_QUESTION_ID); - FeedbackQuestionAttributes feedbackQuestion = logic.getFeedbackQuestion(feedbackQuestionId); - if (feedbackQuestion == null) { - throw new EntityNotFoundException("The feedback question does not exist."); + + FeedbackQuestion feedbackQuestion = null; + FeedbackQuestionAttributes feedbackQuestionAttributes = null; + String courseId = null; + + try { + UUID feedbackQuestionSqlId = getUuidFromString(Const.ParamsNames.FEEDBACK_QUESTION_ID, feedbackQuestionId); + feedbackQuestion = sqlLogic.getFeedbackQuestion(feedbackQuestionSqlId); + + if (feedbackQuestion == null) { + throw new EntityNotFoundException("The feedback question does not exist."); + } + + courseId = feedbackQuestion.getCourseId(); + } catch (InvalidHttpParameterException verifyHttpParameterFailure) { + // if the question id cannot be converted to UUID, we check the datastore for the question + feedbackQuestionAttributes = logic.getFeedbackQuestion(feedbackQuestionId); + + if (feedbackQuestionAttributes == null) { + throw new EntityNotFoundException("The feedback question does not exist."); + } + + courseId = feedbackQuestionAttributes.getCourseId(); } - FeedbackSessionAttributes feedbackSession = - getNonNullFeedbackSession(feedbackQuestion.getFeedbackSessionName(), feedbackQuestion.getCourseId()); + if (!isCourseMigrated(courseId)) { + handleDataStoreAccessControl(feedbackQuestionAttributes); + return; + } + + FeedbackSession feedbackSession = feedbackQuestion.getFeedbackSession(); verifyInstructorCanSeeQuestionIfInModeration(feedbackQuestion); verifyNotPreview(); @@ -57,7 +88,45 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException { switch (intent) { case STUDENT_SUBMISSION: gateKeeper.verifyAnswerableForStudent(feedbackQuestion); - StudentAttributes studentAttributes = getStudentOfCourseFromRequest(feedbackQuestion.getCourseId()); + Student student = getSqlStudentOfCourseFromRequest(feedbackQuestion.getCourseId()); + if (student == null) { + throw new EntityNotFoundException("Student does not exist."); + } + feedbackSession = feedbackSession.getCopyForUser(student.getEmail()); + verifySessionOpenExceptForModeration(feedbackSession); + checkAccessControlForStudentFeedbackSubmission(student, feedbackSession); + break; + case INSTRUCTOR_SUBMISSION: + gateKeeper.verifyAnswerableForInstructor(feedbackQuestion); + Instructor instructor = getSqlInstructorOfCourseFromRequest(feedbackQuestion.getCourseId()); + if (instructor == null) { + throw new EntityNotFoundException("Instructor does not exist."); + } + feedbackSession = feedbackSession.getCopyForUser(instructor.getEmail()); + verifySessionOpenExceptForModeration(feedbackSession); + checkAccessControlForInstructorFeedbackSubmission(instructor, feedbackSession); + break; + case INSTRUCTOR_RESULT: + case STUDENT_RESULT: + throw new InvalidHttpParameterException("Invalid intent for this action"); + default: + throw new InvalidHttpParameterException("Unknown intent " + intent); + } + } + + private void handleDataStoreAccessControl(FeedbackQuestionAttributes feedbackQuestionAttributes) + throws EntityNotFoundException, InvalidHttpParameterException, UnauthorizedAccessException { + FeedbackSessionAttributes feedbackSession = + getNonNullFeedbackSession(feedbackQuestionAttributes.getFeedbackSessionName(), feedbackQuestionAttributes.getCourseId()); + + verifyInstructorCanSeeQuestionIfInModeration(feedbackQuestionAttributes); + verifyNotPreview(); + + Intent intent = Intent.valueOf(getNonNullRequestParamValue(Const.ParamsNames.INTENT)); + switch (intent) { + case STUDENT_SUBMISSION: + gateKeeper.verifyAnswerableForStudent(feedbackQuestionAttributes); + StudentAttributes studentAttributes = getStudentOfCourseFromRequest(feedbackQuestionAttributes.getCourseId()); if (studentAttributes == null) { throw new EntityNotFoundException("Student does not exist."); } @@ -66,8 +135,8 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException { checkAccessControlForStudentFeedbackSubmission(studentAttributes, feedbackSession); break; case INSTRUCTOR_SUBMISSION: - gateKeeper.verifyAnswerableForInstructor(feedbackQuestion); - InstructorAttributes instructorAttributes = getInstructorOfCourseFromRequest(feedbackQuestion.getCourseId()); + gateKeeper.verifyAnswerableForInstructor(feedbackQuestionAttributes); + InstructorAttributes instructorAttributes = getInstructorOfCourseFromRequest(feedbackQuestionAttributes.getCourseId()); if (instructorAttributes == null) { throw new EntityNotFoundException("Instructor does not exist."); } @@ -86,11 +155,176 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException { @Override public JsonResult execute() throws InvalidHttpRequestBodyException, InvalidOperationException { String feedbackQuestionId = getNonNullRequestParamValue(Const.ParamsNames.FEEDBACK_QUESTION_ID); - FeedbackQuestionAttributes feedbackQuestion = logic.getFeedbackQuestion(feedbackQuestionId); - if (feedbackQuestion == null) { - throw new EntityNotFoundException("The feedback question does not exist."); + + FeedbackQuestion feedbackQuestionSQL = null; + FeedbackQuestionAttributes feedbackQuestionAttributes = null; + String courseId = null; + + try { + UUID feedbackQuestionSqlId = getUuidFromString(Const.ParamsNames.FEEDBACK_QUESTION_ID, feedbackQuestionId); + feedbackQuestionSQL = sqlLogic.getFeedbackQuestion(feedbackQuestionSqlId); + + if (feedbackQuestionSQL == null) { + throw new EntityNotFoundException("The feedback question does not exist."); + } + + courseId = feedbackQuestionSQL.getCourseId(); + } catch (InvalidHttpParameterException verifyHttpParameterFailure) { + // if the question id cannot be converted to UUID, we check the datastore for the question + feedbackQuestionAttributes = logic.getFeedbackQuestion(feedbackQuestionId); + + if (feedbackQuestionAttributes == null) { + throw new EntityNotFoundException("The feedback question does not exist."); + } + + courseId = feedbackQuestionAttributes.getCourseId(); + } + + + if (!isCourseMigrated(courseId)) { + return handleDataStoreExecute(feedbackQuestionAttributes); + } + + final FeedbackQuestion feedbackQuestion = feedbackQuestionSQL; + List existingResponses; + Map recipientsOfTheQuestion; + + String giverIdentifier; + Section giverSection; + Intent intent = Intent.valueOf(getNonNullRequestParamValue(Const.ParamsNames.INTENT)); + switch (intent) { + case STUDENT_SUBMISSION: + Student student = getSqlStudentOfCourseFromRequest(feedbackQuestion.getCourseId()); + giverIdentifier = + feedbackQuestion.getGiverType() == FeedbackParticipantType.TEAMS + ? student.getTeamName() : student.getEmail(); + giverSection = student.getSection(); + existingResponses = sqlLogic.getFeedbackResponsesFromStudentOrTeamForQuestion(feedbackQuestion, student); + recipientsOfTheQuestion = sqlLogic.getRecipientsOfQuestion(feedbackQuestion, null, student); + sqlLogic.populateFieldsToGenerateInQuestion(feedbackQuestion, + feedbackQuestion.getCourseId(), student.getEmail(), student.getTeamName()); + break; + case INSTRUCTOR_SUBMISSION: + Instructor instructor = getSqlInstructorOfCourseFromRequest(feedbackQuestion.getCourseId()); + giverIdentifier = instructor.getEmail(); + giverSection = Const.DEFAULT_SQL_SECTION; + existingResponses = sqlLogic.getFeedbackResponsesFromInstructorForQuestion(feedbackQuestion, instructor); + recipientsOfTheQuestion = sqlLogic.getRecipientsOfQuestion(feedbackQuestion, instructor, null); + sqlLogic.populateFieldsToGenerateInQuestion(feedbackQuestion, + feedbackQuestion.getCourseId(), instructor.getEmail(), null); + break; + default: + throw new InvalidHttpParameterException("Unknown intent " + intent); } + Map existingResponsesPerRecipient = new HashMap<>(); + existingResponses.forEach(response -> existingResponsesPerRecipient.put(response.getRecipient(), response)); + + FeedbackResponsesRequest submitRequest = getAndValidateRequestBody(FeedbackResponsesRequest.class); + log.info(JsonUtils.toCompactJson(submitRequest)); + + for (String recipient : submitRequest.getRecipients()) { + if (!recipientsOfTheQuestion.containsKey(recipient)) { + throw new InvalidOperationException( + "The recipient " + recipient + " is not a valid recipient of the question"); + } + } + + List feedbackResponsesToValidate = new ArrayList<>(); + List feedbackResponsesToAdd = new ArrayList<>(); + List feedbackResponsesToUpdate = new ArrayList<>(); + + submitRequest.getResponses().forEach(responseRequest -> { + String recipient = responseRequest.getRecipient(); + FeedbackResponseDetails responseDetails = responseRequest.getResponseDetails(); + + if (existingResponsesPerRecipient.containsKey(recipient)) { + Section recipientSection = getRecipientSection(feedbackQuestion.getCourseId(), + feedbackQuestion.getGiverType(), + feedbackQuestion.getRecipientType(), recipient); + + FeedbackResponse existingFeedbackResponse = existingResponsesPerRecipient.get(recipient); + FeedbackResponse updatedFeedbackResponse = FeedbackResponse.updateResponse( + existingFeedbackResponse, + feedbackQuestion, + giverIdentifier, + giverSection, + recipient, + recipientSection, + responseDetails); + + feedbackResponsesToValidate.add(updatedFeedbackResponse); + feedbackResponsesToUpdate.add(updatedFeedbackResponse); + } else { + FeedbackResponse feedbackResponse = FeedbackResponse.makeResponse( + feedbackQuestion, + giverIdentifier, + giverSection, + recipient, + getRecipientSection(feedbackQuestion.getCourseId(), + feedbackQuestion.getGiverType(), + feedbackQuestion.getRecipientType(), recipient), + responseDetails + ); + + feedbackResponsesToValidate.add(feedbackResponse); + feedbackResponsesToAdd.add(feedbackResponse); + } + }); + + List responseDetails = feedbackResponsesToValidate.stream() + .map(FeedbackResponse::getFeedbackResponseDetailsCopy) + .collect(Collectors.toList()); + + int numRecipients = feedbackQuestion.getNumOfEntitiesToGiveFeedbackTo(); + if (numRecipients == Const.MAX_POSSIBLE_RECIPIENTS + || numRecipients > recipientsOfTheQuestion.size()) { + numRecipients = recipientsOfTheQuestion.size(); + } + + List questionSpecificErrors = + feedbackQuestion.getQuestionDetailsCopy() + .validateResponsesDetails(responseDetails, numRecipients); + + if (!questionSpecificErrors.isEmpty()) { + throw new InvalidHttpRequestBodyException(questionSpecificErrors.toString()); + } + + List recipients = submitRequest.getRecipients(); + List feedbackResponsesToDelete = existingResponsesPerRecipient.entrySet().stream() + .filter(entry -> !recipients.contains(entry.getKey())) + .map(entry -> entry.getValue()) + .collect(Collectors.toList()); + + for (FeedbackResponse feedbackResponse : feedbackResponsesToDelete) { + sqlLogic.deleteFeedbackResponsesAndCommentsCascade(feedbackResponse); + } + + List output = new ArrayList<>(); + + for (FeedbackResponse feedbackResponse : feedbackResponsesToAdd) { + try { + output.add(sqlLogic.createFeedbackResponse(feedbackResponse)); + } catch (InvalidParametersException | EntityAlreadyExistsException e) { + // None of the exceptions should be happening as the responses have been pre-validated + log.severe("Encountered exception when creating response: " + e.getMessage(), e); + } + } + + for (FeedbackResponse feedbackResponse : feedbackResponsesToUpdate) { + try { + output.add(sqlLogic.updateFeedbackResponseCascade(feedbackResponse)); + } catch (InvalidParametersException | EntityAlreadyExistsException | EntityDoesNotExistException e) { + // None of the exceptions should be happening as the responses have been pre-validated + log.severe("Encountered exception when updating response: " + e.getMessage(), e); + } + } + + return new JsonResult(FeedbackResponsesData.createFromEntity(output)); + } + + private JsonResult handleDataStoreExecute(FeedbackQuestionAttributes feedbackQuestion) + throws InvalidHttpRequestBodyException, InvalidOperationException { List existingResponses; Map recipientsOfTheQuestion; @@ -144,7 +378,7 @@ public JsonResult execute() throws InvalidHttpRequestBodyException, InvalidOpera FeedbackResponseDetails responseDetails = responseRequest.getResponseDetails(); if (existingResponsesPerRecipient.containsKey(recipient)) { - String recipientSection = getRecipientSection(feedbackQuestion.getCourseId(), + String recipientSection = getDatastoreRecipientSection(feedbackQuestion.getCourseId(), feedbackQuestion.getGiverType(), feedbackQuestion.getRecipientType(), recipient); FeedbackResponseAttributes updatedResponse = @@ -165,7 +399,7 @@ public JsonResult execute() throws InvalidHttpRequestBodyException, InvalidOpera FeedbackResponseAttributes feedbackResponse = FeedbackResponseAttributes .builder(feedbackQuestion.getId(), giverIdentifier, recipient) .withGiverSection(giverSection) - .withRecipientSection(getRecipientSection(feedbackQuestion.getCourseId(), + .withRecipientSection(getDatastoreRecipientSection(feedbackQuestion.getCourseId(), feedbackQuestion.getGiverType(), feedbackQuestion.getRecipientType(), recipient)) .withCourseId(feedbackQuestion.getCourseId()) @@ -226,7 +460,6 @@ public JsonResult execute() throws InvalidHttpRequestBodyException, InvalidOpera } } - return new JsonResult(new FeedbackResponsesData(output)); + return new JsonResult(FeedbackResponsesData.createFromAttributes(output)); } - -} +} \ No newline at end of file