From 34248a030281645be07aa68974dfc8265be6c0d1 Mon Sep 17 00:00:00 2001 From: FergusMok Date: Thu, 2 May 2024 15:10:04 +0800 Subject: [PATCH] [#12048] Migrate SubmitFeedbackResponseAction tests (#13033) * Skeleton till shouldAllowIfBeforeDeadline * Save progress * Save progress * Fix all tests * Fix linting * Remove redundant * Refactor to fit integration tests structure * Add breaking changes * Add flush * Fix linting * Fix clean-up from access control --- .../core/FeedbackQuestionsLogicIT.java | 5 +- .../storage/sqlapi/FeedbackQuestionsDbIT.java | 8 +- .../SubmitFeedbackResponsesActionIT.java | 727 ++++++++++++++++++ src/it/resources/data/typicalDataBundle.json | 88 ++- .../java/teammates/sqllogic/api/Logic.java | 30 +- .../core/DeadlineExtensionsLogic.java | 11 +- .../sqllogic/core/FeedbackQuestionsLogic.java | 7 + .../sqllogic/core/FeedbackResponsesLogic.java | 34 +- .../storage/sqlapi/FeedbackQuestionsDb.java | 16 + .../storage/sqlapi/FeedbackResponsesDb.java | 21 + .../storage/sqlentity/FeedbackSession.java | 8 +- .../webapi/BasicFeedbackSubmissionAction.java | 10 +- .../CreateFeedbackResponseCommentAction.java | 4 +- .../DeleteFeedbackResponseCommentAction.java | 4 +- .../webapi/SubmitFeedbackResponsesAction.java | 8 +- .../UpdateFeedbackResponseCommentAction.java | 4 +- .../SubmitFeedbackResponsesActionTest.java | 4 +- 17 files changed, 934 insertions(+), 55 deletions(-) create mode 100644 src/it/java/teammates/it/ui/webapi/SubmitFeedbackResponsesActionIT.java diff --git a/src/it/java/teammates/it/sqllogic/core/FeedbackQuestionsLogicIT.java b/src/it/java/teammates/it/sqllogic/core/FeedbackQuestionsLogicIT.java index ee5706ac091..ddbc538f6f3 100644 --- a/src/it/java/teammates/it/sqllogic/core/FeedbackQuestionsLogicIT.java +++ b/src/it/java/teammates/it/sqllogic/core/FeedbackQuestionsLogicIT.java @@ -75,8 +75,11 @@ public void testGetFeedbackQuestionsForSession() { FeedbackQuestion fq4 = typicalDataBundle.feedbackQuestions.get("qn4InSession1InCourse1"); FeedbackQuestion fq5 = typicalDataBundle.feedbackQuestions.get("qn5InSession1InCourse1"); FeedbackQuestion fq6 = typicalDataBundle.feedbackQuestions.get("qn6InSession1InCourse1NoResponses"); + FeedbackQuestion fq7 = typicalDataBundle.feedbackQuestions.get("qn7InSession1InCourse1"); + FeedbackQuestion fq8 = typicalDataBundle.feedbackQuestions.get("qn8InSession1InCourse1"); + FeedbackQuestion fq9 = typicalDataBundle.feedbackQuestions.get("qn9InSession1InCourse1"); - List expectedQuestions = List.of(fq1, fq2, fq3, fq4, fq5, fq6); + List expectedQuestions = List.of(fq1, fq2, fq3, fq4, fq5, fq6, fq7, fq8, fq9); List actualQuestions = fqLogic.getFeedbackQuestionsForSession(fs); diff --git a/src/it/java/teammates/it/storage/sqlapi/FeedbackQuestionsDbIT.java b/src/it/java/teammates/it/storage/sqlapi/FeedbackQuestionsDbIT.java index 145f4ee4ef0..b201263775c 100644 --- a/src/it/java/teammates/it/storage/sqlapi/FeedbackQuestionsDbIT.java +++ b/src/it/java/teammates/it/storage/sqlapi/FeedbackQuestionsDbIT.java @@ -93,8 +93,11 @@ public void testGetFeedbackQuestionsForSession() { FeedbackQuestion fq4 = typicalDataBundle.feedbackQuestions.get("qn4InSession1InCourse1"); FeedbackQuestion fq5 = typicalDataBundle.feedbackQuestions.get("qn5InSession1InCourse1"); FeedbackQuestion fq6 = typicalDataBundle.feedbackQuestions.get("qn6InSession1InCourse1NoResponses"); + FeedbackQuestion fq7 = typicalDataBundle.feedbackQuestions.get("qn7InSession1InCourse1"); + FeedbackQuestion fq8 = typicalDataBundle.feedbackQuestions.get("qn8InSession1InCourse1"); + FeedbackQuestion fq9 = typicalDataBundle.feedbackQuestions.get("qn9InSession1InCourse1"); - List expectedQuestions = List.of(fq1, fq2, fq3, fq4, fq5, fq6); + List expectedQuestions = List.of(fq1, fq2, fq3, fq4, fq5, fq6, fq7, fq8, fq9); List actualQuestions = fqDb.getFeedbackQuestionsForSession(fs.getId()); @@ -112,8 +115,9 @@ public void testGetFeedbackQuestionsForGiverType() { FeedbackSession fs = typicalDataBundle.feedbackSessions.get("session1InCourse1"); FeedbackQuestion fq1 = typicalDataBundle.feedbackQuestions.get("qn1InSession1InCourse1"); FeedbackQuestion fq2 = typicalDataBundle.feedbackQuestions.get("qn2InSession1InCourse1"); + FeedbackQuestion fq9 = typicalDataBundle.feedbackQuestions.get("qn9InSession1InCourse1"); - List expectedQuestions = List.of(fq1, fq2); + List expectedQuestions = List.of(fq1, fq2, fq9); List actualQuestions = fqDb.getFeedbackQuestionsForGiverType(fs, FeedbackParticipantType.STUDENTS); diff --git a/src/it/java/teammates/it/ui/webapi/SubmitFeedbackResponsesActionIT.java b/src/it/java/teammates/it/ui/webapi/SubmitFeedbackResponsesActionIT.java new file mode 100644 index 00000000000..fa278a379b3 --- /dev/null +++ b/src/it/java/teammates/it/ui/webapi/SubmitFeedbackResponsesActionIT.java @@ -0,0 +1,727 @@ +package teammates.it.ui.webapi; + +import java.time.Instant; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; + +import org.apache.commons.lang.StringEscapeUtils; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import teammates.common.datatransfer.InstructorPrivileges; +import teammates.common.datatransfer.questions.FeedbackResponseDetails; +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.HibernateUtil; +import teammates.common.util.SanitizationHelper; +import teammates.common.util.TimeHelper; +import teammates.storage.sqlentity.DeadlineExtension; +import teammates.storage.sqlentity.FeedbackQuestion; +import teammates.storage.sqlentity.FeedbackResponse; +import teammates.storage.sqlentity.FeedbackSession; +import teammates.storage.sqlentity.Instructor; +import teammates.storage.sqlentity.Student; +import teammates.storage.sqlentity.User; +import teammates.ui.output.FeedbackResponseData; +import teammates.ui.output.FeedbackResponsesData; +import teammates.ui.request.FeedbackResponsesRequest; +import teammates.ui.request.Intent; +import teammates.ui.webapi.JsonResult; +import teammates.ui.webapi.SubmitFeedbackResponsesAction; + +/** + * SUT: {@link SubmitFeedbackResponsesAction}. + */ +public class SubmitFeedbackResponsesActionIT extends BaseActionIT { + @Override + @BeforeMethod + protected void setUp() throws Exception { + super.setUp(); + persistDataBundle(typicalBundle); + HibernateUtil.flushSession(); + } + + @Override + protected String getActionUri() { + return Const.ResourceURIs.RESPONSES; + } + + @Override + protected String getRequestMethod() { + return PUT; + } + + private FeedbackSession getSession(String sessionId) { + return typicalBundle.feedbackSessions.get(sessionId); + } + + private Instructor getInstructor(String instructorId) { + return typicalBundle.instructors.get(instructorId); + } + + private Instructor loginInstructor(String instructorId) { + Instructor instructor = getInstructor(instructorId); + loginAsInstructor(instructor.getGoogleId()); + HibernateUtil.flushSession(); + return instructor; + } + + private Student getStudent(String studentId) { + return typicalBundle.students.get(studentId); + } + + private List getStudents(String... studentIds) { + List students = new ArrayList<>(); + for (String studentId : studentIds) { + Student student = getStudent(studentId); + students.add(student); + } + + return students; + } + + private Student loginStudent(String studentId) { + Student student = getStudent(studentId); + loginAsStudent(student.getGoogleId()); + HibernateUtil.flushSession(); + return student; + } + + private FeedbackQuestion getQuestion( + FeedbackSession session, int questionNumber) { + return logic.getFeedbackQuestionForSessionQuestionNumber(session.getId(), questionNumber); + } + + private void setStartTime(FeedbackSession session, int days) + throws InvalidParametersException, EntityDoesNotExistException { + Instant startTime = TimeHelper.getInstantDaysOffsetFromNow(days); + + session.setStartTime(startTime); + + logic.updateFeedbackSession(session); + HibernateUtil.flushSession(); + } + + private void setEndTime(FeedbackSession session, int days) + throws InvalidParametersException, EntityDoesNotExistException { + Instant endTime = TimeHelper.getInstantDaysOffsetFromNow(days); + + session.setEndTime(endTime); + + logic.updateFeedbackSession(session); + HibernateUtil.flushSession(); + } + + private void setUserDeadlineExtension(FeedbackSession session, User user, int days) + throws InvalidParametersException, EntityDoesNotExistException, EntityAlreadyExistsException { + DeadlineExtension newDeadline = + new DeadlineExtension(user, session, TimeHelper.getInstantDaysOffsetFromNow(days)); + + newDeadline.setFeedbackSession(session); + newDeadline.setUser(user); + + DeadlineExtension existingDeadlineEndTime = logic.getDeadlineExtensionEntityForUser(session, user); + if (existingDeadlineEndTime != null) { + newDeadline.setId(existingDeadlineEndTime.getId()); + logic.updateDeadlineExtension(newDeadline); + } else { + logic.createDeadlineExtension(newDeadline); + } + } + + private void deleteDeadlineExtensionForUser(FeedbackSession session, User user) { + DeadlineExtension existingDeadlineEndTime = logic.getDeadlineExtensionEntityForUser(session, user); + if (existingDeadlineEndTime == null) { + return; + } + + logic.deleteDeadlineExtension(existingDeadlineEndTime); + } + + private String[] buildSubmissionParams(FeedbackSession session, int questionNumber, Intent intent) { + FeedbackQuestion question = getQuestion(session, questionNumber); + return buildSubmissionParams(question, intent); + } + + private String[] buildSubmissionParams(FeedbackQuestion question, Intent intent) { + String questionId = question != null ? question.getId().toString() : ""; + + return new String[] {Const.ParamsNames.FEEDBACK_QUESTION_ID, questionId, Const.ParamsNames.INTENT, + intent.toString()}; + } + + private void setSubmitSessionInSectionsInstructorPrivilege(FeedbackSession session, + Instructor instructor, boolean value) + throws Exception { + String courseId = session.getCourseId(); + + InstructorPrivileges instructorPrivileges = new InstructorPrivileges(); + instructorPrivileges.updatePrivilege(Const.InstructorPermissions.CAN_SUBMIT_SESSION_IN_SECTIONS, value); + + instructor.getCourse().setId(courseId); + instructor.setPrivileges(instructorPrivileges); + + logic.updateToEnsureValidityOfInstructorsForTheCourse(courseId, instructor); + } + + private List extractStudentEmails(List students) { + return students.stream().map(recipient -> recipient.getEmail()).collect(Collectors.toList()); + } + + private List extractStudentTeams(List students) { + return students.stream().map(recipient -> recipient.getTeam().getName()).collect(Collectors.toList()); + } + + private FeedbackResponsesRequest buildRequestBodyWithStudentRecipientsEmail( + List recipients) { + List emails = extractStudentEmails(recipients); + return buildRequestBody(emails); + } + + private FeedbackResponsesRequest buildRequestBodyWithStudentRecipientsTeam( + List recipients) { + List teams = extractStudentTeams(recipients); + return buildRequestBody(teams); + } + + private List extractInstructorEmails( + List students) { + return students.stream().map(recipient -> recipient.getEmail()).collect(Collectors.toList()); + } + + private FeedbackResponsesRequest buildRequestBodyWithInstructorRecipients(List recipients) { + List emails = extractInstructorEmails(recipients); + return buildRequestBody(emails); + } + + private FeedbackResponsesRequest buildRequestBody(List values) { + List responses = new ArrayList<>(); + + for (String value : values) { + FeedbackTextResponseDetails responseDetails = new FeedbackTextResponseDetails("Response for " + value); + FeedbackResponsesRequest.FeedbackResponseRequest response = + new FeedbackResponsesRequest.FeedbackResponseRequest(value, responseDetails); + + responses.add(response); + } + + FeedbackResponsesRequest requestBody = new FeedbackResponsesRequest(); + requestBody.setResponses(responses); + return requestBody; + } + + private List callExecute(FeedbackResponsesRequest requestBody, String[] submissionParams) { + SubmitFeedbackResponsesAction action = getAction(requestBody, submissionParams); + JsonResult result = getJsonResult(action); + + FeedbackResponsesData output = (FeedbackResponsesData) result.getOutput(); + return output.getResponses(); + } + + private void validateOutputForStudentRecipientsByEmail(List responses, String giverEmail, + List recipients) { + int responsesSize = responses.size(); + assertEquals(recipients.size(), responsesSize); + + List recipientEmails = extractStudentEmails(recipients); + + validateOutput(responses, giverEmail, recipientEmails); + } + + private void validateOutputForStudentRecipientsByTeam(List responses, String giverTeam, + List recipients) { + int responsesSize = responses.size(); + assertEquals(recipients.size(), responsesSize); + + List recipientTeams = extractStudentTeams(recipients); + + validateOutput(responses, giverTeam, recipientTeams); + } + + private void validateOutputForInstructorRecipients(List responses, String giverEmail, + List recipients) { + int responsesSize = responses.size(); + assertEquals(recipients.size(), responsesSize); + + List recipientEmails = extractInstructorEmails(recipients); + + validateOutput(responses, giverEmail, recipientEmails); + } + + private void validateOutput(List responses, String giverValue, List recipientValues) { + for (int i = 0; i < recipientValues.size(); i++) { + FeedbackResponseData response = responses.get(i); + String recipientValue = recipientValues.get(i); + + assertEquals(giverValue, response.getGiverIdentifier()); + assertEquals(recipientValue, response.getRecipientIdentifier()); + + FeedbackResponseDetails responseDetails = response.getResponseDetails(); + assertEquals(StringEscapeUtils.unescapeHtml( + SanitizationHelper.sanitizeForRichText("Response for " + recipientValue)), + StringEscapeUtils.unescapeHtml(responseDetails.getAnswerString())); + } + } + + private void validateStudentDatabaseByTeam(FeedbackSession session, FeedbackQuestion question, + String giverTeam, List recipients) { + List studentTeamNames = extractStudentTeams(recipients); + + validateDatabaseWithRecipientEmails(session, question, giverTeam, studentTeamNames); + } + + private void validateStudentDatabaseByEmail(FeedbackSession session, FeedbackQuestion question, String giverEmail, + List recipients) { + List studentRecipientEmails = extractStudentEmails(recipients); + + validateDatabaseWithRecipientEmails(session, question, giverEmail, studentRecipientEmails); + } + + private void validateInstructorDatabaseByEmail( + FeedbackSession session, + FeedbackQuestion question, + String giverEmail, List recipients) { + List instructorRecipientEmails = extractInstructorEmails(recipients); + + validateDatabaseWithRecipientEmails(session, question, giverEmail, instructorRecipientEmails); + } + + private void validateDatabaseWithRecipientEmails(FeedbackSession session, FeedbackQuestion feedbackQuestion, + String giverEmail, List recipientEmails) { + + for (String recipientEmail : recipientEmails) { + List feedbackResponses = + logic.getFeedbackResponsesFromGiverAndRecipientForCourse( + session.getCourseId(), giverEmail, recipientEmail); + + for (FeedbackResponse feedbackResponse : feedbackResponses) { + FeedbackQuestion frFeedbackQuestion = feedbackResponse.getFeedbackQuestion(); + + assertEquals(frFeedbackQuestion, feedbackQuestion); + assertEquals(feedbackResponse.getGiver(), giverEmail); + assertEquals(feedbackResponse.getRecipient(), recipientEmail); + + assertEquals(session.getName(), feedbackQuestion.getFeedbackSessionName()); + assertEquals(session.getCourseId(), feedbackQuestion.getCourseId()); + + FeedbackResponseDetails responseDetails = feedbackResponse.getFeedbackResponseDetailsCopy(); + assertEquals( + StringEscapeUtils.unescapeHtml( + SanitizationHelper.sanitizeForRichText("Response for " + recipientEmail)), + StringEscapeUtils.unescapeHtml(responseDetails.getAnswerString())); + } + } + } + + @Override + @Test + protected void testAccessControl() throws Exception { + FeedbackSession session = getSession("session1InCourse1"); + Student student = loginStudent("student1InCourse1"); + Instructor instructor = loginInstructor("instructor1OfCourse1"); + deleteDeadlineExtensionForUser(session, instructor); + deleteDeadlineExtensionForUser(session, student); + + ______TS("Typical case with instructors: feedback question exists"); + setStartTime(session, -1); + setEndTime(session, 1); + + int questionNumber = 4; + String[] submissionParams = buildSubmissionParams(session, questionNumber, Intent.INSTRUCTOR_SUBMISSION); + + verifyCanAccess(submissionParams); + + ______TS("Typical case with student: feedback question exists"); + loginStudent("student1InCourse1"); + setStartTime(session, -1); + setEndTime(session, 3); + + questionNumber = 2; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.STUDENT_SUBMISSION); + + verifyCanAccess(submissionParams); + + ______TS("Failure with instructors: no feedback question parameter"); + loginInstructor("instructor1OfCourse1"); + setStartTime(session, -1); + setEndTime(session, 3); + + submissionParams = new String[] {Const.ParamsNames.INTENT, Intent.INSTRUCTOR_SUBMISSION.toString()}; + + verifyHttpParameterFailureAcl(submissionParams); + + ______TS("Failure with instructors: feedback question does not exist"); + setStartTime(session, -1); + setEndTime(session, 3); + + questionNumber = 222; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.INSTRUCTOR_SUBMISSION); + + verifyEntityNotFoundAcl(submissionParams); + + ______TS("Failure with students: no feedback question parameter"); + loginStudent("student1InCourse1"); + setStartTime(session, -1); + setEndTime(session, 3); + + questionNumber = 2; + FeedbackQuestion question = getQuestion(session, questionNumber); + submissionParams = new String[] {Const.ParamsNames.FEEDBACK_QUESTION_ID, question.getId().toString()}; + + verifyHttpParameterFailureAcl(submissionParams); + + ______TS("Failure with students: invalid intent for action STUDENT_RESULT"); + setStartTime(session, -1); + setEndTime(session, 3); + + questionNumber = 2; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.STUDENT_RESULT); + + verifyHttpParameterFailureAcl(submissionParams); + + ______TS("Failure with students: submission not open, start time is in the future"); + loginStudent("student1InCourse1"); + setStartTime(session, 2); + setEndTime(session, 3); + + questionNumber = 2; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.STUDENT_SUBMISSION); + + verifyCannotAccess(submissionParams); + + ______TS("Typical success with students: redundant deadline extension"); + setStartTime(session, -1); + setEndTime(session, 3); + + questionNumber = 2; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.STUDENT_SUBMISSION); + + // No selective deadline, should pass. + verifyCanAccess(submissionParams); + + // With selective deadline, should still pass. + setUserDeadlineExtension(session, student, 7); + verifyCanAccess(submissionParams); + + ______TS("Typical success with instructor: deadline extension granted after submission closed"); + loginInstructor("instructor1OfCourse1"); + setStartTime(session, -3); + setEndTime(session, -2); + + questionNumber = 4; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.INSTRUCTOR_SUBMISSION); + setUserDeadlineExtension(session, instructor, -1); + + // No selective deadline, should not pass. + verifyCannotAccess(submissionParams); + + // With selective deadline, should pass + setUserDeadlineExtension(session, instructor, 1); + verifyCanAccess(submissionParams); + + ______TS("Failure with instructor: submission after deadline extension expired"); + setStartTime(session, -3); + setEndTime(session, -2); + + questionNumber = 4; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.INSTRUCTOR_SUBMISSION); + + setUserDeadlineExtension(session, instructor, -1); + verifyCannotAccess(submissionParams); + + ______TS("Typical success with student: student answers question with correct giver"); + loginStudent("student1InCourse1"); + deleteDeadlineExtensionForUser(session, student); + setEndTime(session, 3); + setStartTime(session, -1); + + questionNumber = 2; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.STUDENT_SUBMISSION); + + verifyCanAccess(submissionParams); + + ______TS("Failure with student: student answers question with incorrect giver"); + setStartTime(session, -1); + setEndTime(session, 3); + + questionNumber = 4; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.STUDENT_SUBMISSION); + + verifyCannotAccess(submissionParams); + + ______TS("Failure with student: student logged out"); + logoutUser(); + setStartTime(session, -1); + setEndTime(session, 3); + + questionNumber = 2; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.STUDENT_SUBMISSION); + + verifyCannotAccess(submissionParams); + + ______TS("Failure with student: student logged in as instructor"); + logoutUser(); + loginInstructor("instructor1OfCourse1"); + setStartTime(session, -1); + setEndTime(session, 3); + + questionNumber = 2; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.STUDENT_SUBMISSION); + + verifyCannotAccess(submissionParams); + + loginStudent("student1InCourse1"); + verifyCanAccess(submissionParams); + + ______TS("Failure with student: student logged in as admin"); + logoutUser(); + loginAsAdmin(); + setStartTime(session, -1); + setEndTime(session, 3); + + questionNumber = 2; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.STUDENT_SUBMISSION); + + verifyCannotAccess(submissionParams); + + loginStudent("student1InCourse1"); + verifyCanAccess(submissionParams); + + ______TS("Typical success with student: logged in as admin masquerading as student"); + logoutUser(); + loginAsAdmin(); + setStartTime(session, -1); + setEndTime(session, 3); + + questionNumber = 2; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.STUDENT_SUBMISSION); + + verifyCanMasquerade(student.getGoogleId(), submissionParams); + + ______TS("Typical success with instructor: instructor answers question with correct giver"); + loginInstructor("instructor1OfCourse1"); + setStartTime(session, -1); + setEndTime(session, 3); + + questionNumber = 4; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.INSTRUCTOR_SUBMISSION); + + verifyCanAccess(submissionParams); + + ______TS("Typical success with instructor: instructor answers question to self-answerable question"); + loginInstructor("instructor1OfCourse1"); + setStartTime(session, -1); + setEndTime(session, 3); + + questionNumber = 3; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.INSTRUCTOR_SUBMISSION); + + verifyCanAccess(submissionParams); + + ______TS("Failure with instructor: instructor answers question with incorrect giver"); + loginInstructor("instructor1OfCourse1"); + setStartTime(session, -1); + setEndTime(session, 3); + + questionNumber = 1; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.INSTRUCTOR_SUBMISSION); + + verifyCannotAccess(submissionParams); + + ______TS("Failure with instructor: instructor logged out"); + logoutUser(); + setStartTime(session, -1); + setEndTime(session, 3); + + questionNumber = 3; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.INSTRUCTOR_SUBMISSION); + + verifyCannotAccess(submissionParams); + + ______TS("Failure with instructor: instructor logged in as admin"); + logoutUser(); + loginAsAdmin(); + setStartTime(session, -1); + setEndTime(session, 3); + + questionNumber = 4; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.INSTRUCTOR_SUBMISSION); + + verifyCannotAccess(submissionParams); + + loginInstructor("instructor1OfCourse1"); + verifyCanAccess(submissionParams); + + ______TS("Typical success with instructor: logged in as admin masquerading as instructor"); + logoutUser(); + loginAsAdmin(); + setStartTime(session, -1); + setEndTime(session, 3); + + questionNumber = 4; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.INSTRUCTOR_SUBMISSION); + + verifyCanMasquerade(instructor.getGoogleId(), submissionParams); + + ______TS("Failure with instructor: instructor logged in as student"); + loginStudent("student1InCourse1"); + setStartTime(session, -1); + setEndTime(session, 3); + + questionNumber = 4; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.INSTRUCTOR_SUBMISSION); + + verifyCannotAccess(submissionParams); + + ______TS("Typical success with instructor: instructor has modify session comment privileges"); + loginInstructor("instructor1OfCourse1"); + setStartTime(session, -1); + setEndTime(session, 3); + + setSubmitSessionInSectionsInstructorPrivilege(session, instructor, true); + + questionNumber = 4; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.INSTRUCTOR_SUBMISSION); + + verifyCanAccess(submissionParams); + verifyCanMasquerade(instructor.getGoogleId(), submissionParams); + + ______TS("Failure with instructor: instructor has no modify session comment privileges"); + loginInstructor("instructor1OfCourse1"); + setStartTime(session, -1); + setEndTime(session, 3); + + setSubmitSessionInSectionsInstructorPrivilege(session, instructor, false); + + questionNumber = 4; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.INSTRUCTOR_SUBMISSION); + + verifyCannotAccess(submissionParams); + verifyCannotMasquerade(instructor.getGoogleId(), submissionParams); + + // Reset privileges + setSubmitSessionInSectionsInstructorPrivilege(session, instructor, true); + } + + @Override + @Test + public void testExecute() { + ______TS("Failure: invalid http parameters"); + loginInstructor("instructor1OfCourse1"); + + verifyHttpParameterFailure(new String[] {}); + + ______TS("Failure: not feedback question parameter specified"); + String[] submissionParams = new String[] {Const.ParamsNames.INTENT, Intent.STUDENT_SUBMISSION.toString()}; + verifyHttpParameterFailure(submissionParams); + + ______TS("Failure: feedback question does not exist"); + submissionParams = new String[] { + Const.ParamsNames.INTENT, Intent.STUDENT_SUBMISSION.toString(), + Const.ParamsNames.FEEDBACK_QUESTION_ID, "non-existent id"}; + verifyEntityNotFound(submissionParams); + + ______TS("Failure: instructor has invalid intent"); + FeedbackSession session = getSession("session1InCourse1"); + int questionNumber = 3; + + submissionParams = buildSubmissionParams(session, questionNumber, Intent.STUDENT_RESULT); + verifyHttpParameterFailure(submissionParams); + + submissionParams = buildSubmissionParams(session, questionNumber, Intent.FULL_DETAIL); + verifyHttpParameterFailure(submissionParams); + + ______TS("Failure: no request body"); + loginStudent("student1InCourse1"); + + questionNumber = 2; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.STUDENT_SUBMISSION); + verifyHttpRequestBodyFailure(null, submissionParams); + + ______TS("Failure: request body has no recipient"); + loginInstructor("instructor1OfCourse1"); + + questionNumber = 4; + submissionParams = buildSubmissionParams(session, questionNumber, Intent.INSTRUCTOR_SUBMISSION); + + // Null recipient + List nullEmail = Collections.singletonList(null); + FeedbackResponsesRequest requestBody = buildRequestBody(nullEmail); + verifyInvalidOperation(requestBody, submissionParams); + + // Empty String recipient + requestBody = buildRequestBody(Collections.singletonList("")); + verifyInvalidOperation(requestBody, submissionParams); + + ______TS("Success: question has no existing responses"); + Instructor instructorGiver = loginInstructor("instructor1OfCourse1"); + + questionNumber = 7; + FeedbackQuestion question = getQuestion(session, questionNumber); + submissionParams = buildSubmissionParams(question, Intent.INSTRUCTOR_SUBMISSION); + + List instructorRecipients = Collections.singletonList(instructorGiver); + requestBody = buildRequestBodyWithInstructorRecipients(instructorRecipients); + + List outputResponses = callExecute(requestBody, submissionParams); + validateOutputForInstructorRecipients(outputResponses, instructorGiver.getEmail(), instructorRecipients); + validateInstructorDatabaseByEmail(session, question, instructorGiver.getEmail(), instructorRecipients); + + ______TS("Success: instructor is a valid giver of the question to student team"); + instructorGiver = loginInstructor("instructor1OfCourse1"); + + questionNumber = 8; + question = getQuestion(session, questionNumber); + submissionParams = buildSubmissionParams(question, Intent.INSTRUCTOR_SUBMISSION); + + List studentRecipients = getStudents("student2InCourse1", "student3InCourse1"); + requestBody = buildRequestBodyWithStudentRecipientsTeam(studentRecipients); + + outputResponses = callExecute(requestBody, submissionParams); + validateOutputForStudentRecipientsByTeam(outputResponses, instructorGiver.getEmail(), studentRecipients); + validateStudentDatabaseByTeam(session, question, instructorGiver.getEmail(), studentRecipients); + + ______TS("Success: question has existing responses"); + Student studentGiver = loginStudent("student1InCourse1"); + + questionNumber = 2; + question = getQuestion(session, questionNumber); + submissionParams = buildSubmissionParams(question, Intent.STUDENT_SUBMISSION); + + studentRecipients = getStudents("student3InCourse1"); + requestBody = buildRequestBodyWithStudentRecipientsEmail(studentRecipients); + + outputResponses = callExecute(requestBody, submissionParams); + validateOutputForStudentRecipientsByEmail(outputResponses, studentGiver.getEmail(), studentRecipients); + validateStudentDatabaseByEmail(session, question, studentGiver.getEmail(), studentRecipients); + + ______TS("Failure: student is a invalid giver of the question"); + questionNumber = 6; + question = getQuestion(session, questionNumber); + submissionParams = buildSubmissionParams(question, Intent.STUDENT_SUBMISSION); + + studentRecipients = getStudents("student2InCourse1"); + requestBody = buildRequestBodyWithStudentRecipientsTeam(studentRecipients); + + verifyInvalidOperation(requestBody, submissionParams); + + ______TS("Success: too many recipients"); + studentGiver = loginStudent("student4InCourse1"); + + questionNumber = 9; + question = getQuestion(session, questionNumber); + submissionParams = buildSubmissionParams(question, Intent.STUDENT_SUBMISSION); + + studentRecipients = getStudents("student2InCourse1", "student3InCourse1"); + requestBody = buildRequestBodyWithStudentRecipientsEmail(studentRecipients); + + outputResponses = callExecute(requestBody, submissionParams); + validateOutputForStudentRecipientsByEmail(outputResponses, studentGiver.getEmail(), studentRecipients); + validateStudentDatabaseByEmail(session, question, studentGiver.getEmail(), studentRecipients); + } +} diff --git a/src/it/resources/data/typicalDataBundle.json b/src/it/resources/data/typicalDataBundle.json index a04d0ef7193..58ac4fa1a51 100644 --- a/src/it/resources/data/typicalDataBundle.json +++ b/src/it/resources/data/typicalDataBundle.json @@ -603,6 +603,21 @@ "name": "student3 In Course1", "comments": "" }, + "student4InCourse1": { + "id": "00000000-0000-4000-8000-000000000611", + "account": { + "id": "00000000-0000-4000-8000-000000000104" + }, + "course": { + "id": "course-1" + }, + "team": { + "id": "00000000-0000-4000-8000-000000000305" + }, + "email": "student4@teammates.tmt", + "name": "student4 In Course1", + "comments": "This student is not in the same team as others in course 1" + }, "student1InCourse2": { "id": "00000000-0000-4000-8000-000000000604", "course": { @@ -689,21 +704,6 @@ "email": "studentOfArchivedCourse@teammates.tmt", "name": "Student In Archived Course", "comments": "" - }, - "student4InCourse1": { - "id": "00000000-0000-4000-8000-000000000611", - "account": { - "id": "00000000-0000-4000-8000-000000000104" - }, - "course": { - "id": "course-1" - }, - "team": { - "id": "00000000-0000-4000-8000-000000000305" - }, - "email": "student4@teammates.tmt", - "name": "student4 In Course1", - "comments": "comment for student4Course1" } }, "feedbackSessions": { @@ -1018,7 +1018,7 @@ "questionType": "TEXT" }, "description": "Feedback question with no responses", - "questionNumber": 5, + "questionNumber": 6, "giverType": "SELF", "recipientType": "NONE", "numOfEntitiesToGiveFeedbackTo": -100, @@ -1026,6 +1026,62 @@ "showGiverNameTo": ["INSTRUCTORS"], "showRecipientNameTo": ["INSTRUCTORS"] }, + "qn7InSession1InCourse1": { + "id": "00000000-0000-4000-8000-000000000807", + "feedbackSession": { + "id": "00000000-0000-4000-8000-000000000701" + }, + "questionDetails": { + "recommendedLength": 100, + "questionText": "This is question 7, a text question.", + "questionType": "TEXT" + }, + "description": "This is a text question.", + "questionNumber": 7, + "giverType": "SELF", + "recipientType": "SELF", + "numOfEntitiesToGiveFeedbackTo": -100, + "showResponsesTo": ["INSTRUCTORS"], + "showGiverNameTo": ["INSTRUCTORS"], + "showRecipientNameTo": ["INSTRUCTORS"] + }, + "qn8InSession1InCourse1": { + "id": "00000000-0000-4000-8000-000000000808", + "feedbackSession": { + "id": "00000000-0000-4000-8000-000000000701" + }, + "questionDetails": { + "recommendedLength": 100, + "questionText": "This is question 8, a text question.", + "questionType": "TEXT" + }, + "description": "This is a text question.", + "questionNumber": 8, + "giverType": "INSTRUCTORS", + "recipientType": "TEAMS_EXCLUDING_SELF", + "numOfEntitiesToGiveFeedbackTo": 1, + "showResponsesTo": ["RECEIVER"], + "showGiverNameTo": ["RECEIVER"], + "showRecipientNameTo": ["RECEIVER"] + }, + "qn9InSession1InCourse1": { + "id": "00000000-0000-4000-8000-000000000809", + "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": 9, + "giverType": "STUDENTS", + "recipientType": "STUDENTS_EXCLUDING_SELF", + "numOfEntitiesToGiveFeedbackTo": 1, + "showResponsesTo": ["INSTRUCTORS"], + "showGiverNameTo": ["INSTRUCTORS"], + "showRecipientNameTo": ["INSTRUCTORS"] + }, "qn1InSession2InCourse1": { "id": "00000000-0000-4000-8001-000000000800", "feedbackSession": { diff --git a/src/main/java/teammates/sqllogic/api/Logic.java b/src/main/java/teammates/sqllogic/api/Logic.java index 659f27750c5..eb2698cd9d7 100644 --- a/src/main/java/teammates/sqllogic/api/Logic.java +++ b/src/main/java/teammates/sqllogic/api/Logic.java @@ -474,7 +474,7 @@ public void deleteDeadlineExtension(DeadlineExtension de) { } /** - * Fetch the deadline extension for a given user and session feedback. + * Fetch the deadline extension end time for a given user and session feedback. * * @return deadline extension instant if exists, else the default end time instant * for the session feedback. @@ -484,7 +484,7 @@ public Instant getDeadlineForUser(FeedbackSession session, User user) { } /** - * Fetch the deadline extension for a given user and session feedback. + * Fetch the deadline extension end time for a given user and session feedback. * * @return deadline extension instant if exists, else return null since no deadline extensions. */ @@ -492,6 +492,15 @@ public Instant getExtendedDeadlineForUser(FeedbackSession session, User user) { return deadlineExtensionsLogic.getExtendedDeadlineForUser(session, user); } + /** + * Fetch the deadline extension entity for a given user and session feedback. + * + * @return deadline extension entity if exists, else return null. + */ + public DeadlineExtension getDeadlineExtensionEntityForUser(FeedbackSession session, User user) { + return deadlineExtensionsLogic.getDeadlineExtensionEntityForUser(session, user); + } + /** * Gets a list of deadline extensions with endTime coming up soon * and possibly need a closing email to be sent. @@ -1305,6 +1314,13 @@ public List getFeedbackQuestionsForSession(FeedbackSession fee return feedbackQuestionsLogic.getFeedbackQuestionsForSession(feedbackSession); } + /** + * Gets the unique feedback question based on sessionId and questionNumber. + */ + public FeedbackQuestion getFeedbackQuestionForSessionQuestionNumber(UUID sessionId, int questionNumber) { + return feedbackQuestionsLogic.getFeedbackQuestionForSessionQuestionNumber(sessionId, questionNumber); + } + /** * Gets a list of all questions for the given session that * students can view/submit. @@ -1599,6 +1615,16 @@ public List getFeedbackResponsesForRecipientForCourse(String c return feedbackResponsesLogic.getFeedbackResponsesForRecipientForCourse(courseId, recipientEmail); } + /** + * Gets all feedback responses from a specific giver and recipient for a course. + */ + public List getFeedbackResponsesFromGiverAndRecipientForCourse(String courseId, String giverEmail, + String recipientEmail) { + + return feedbackResponsesLogic.getFeedbackResponsesFromGiverAndRecipientForCourse(courseId, giverEmail, + recipientEmail); + } + /** * Gets all feedback response comments for a feedback response. */ diff --git a/src/main/java/teammates/sqllogic/core/DeadlineExtensionsLogic.java b/src/main/java/teammates/sqllogic/core/DeadlineExtensionsLogic.java index c77cb255cc2..c27f56f9c5b 100644 --- a/src/main/java/teammates/sqllogic/core/DeadlineExtensionsLogic.java +++ b/src/main/java/teammates/sqllogic/core/DeadlineExtensionsLogic.java @@ -40,7 +40,7 @@ void initLogicDependencies(DeadlineExtensionsDb deadlineExtensionsDb, FeedbackSe } /** - * Get extended deadline for this session and user if it exists, otherwise get the deadline of the session. + * Get extended deadline end time for this session and user if it exists, otherwise get the deadline of the session. */ public Instant getDeadlineForUser(FeedbackSession session, User user) { Instant extendedDeadline = @@ -54,7 +54,7 @@ public Instant getDeadlineForUser(FeedbackSession session, User user) { } /** - * Get extended deadline for this session and user if it exists, otherwise return null. + * Get extended deadline end time for this session and user if it exists, otherwise return null. */ public Instant getExtendedDeadlineForUser(FeedbackSession feedbackSession, User user) { DeadlineExtension deadlineExtension = @@ -66,6 +66,13 @@ public Instant getExtendedDeadlineForUser(FeedbackSession feedbackSession, User return deadlineExtension.getEndTime(); } + /** + * Get deadline entity for this session and user if it exists, otherwise return null. + */ + public DeadlineExtension getDeadlineExtensionEntityForUser(FeedbackSession feedbackSession, User user) { + return deadlineExtensionsDb.getDeadlineExtension(user.getId(), feedbackSession.getId()); + } + /** * Creates a deadline extension. * diff --git a/src/main/java/teammates/sqllogic/core/FeedbackQuestionsLogic.java b/src/main/java/teammates/sqllogic/core/FeedbackQuestionsLogic.java index 7ccf253a0e0..88923417d7d 100644 --- a/src/main/java/teammates/sqllogic/core/FeedbackQuestionsLogic.java +++ b/src/main/java/teammates/sqllogic/core/FeedbackQuestionsLogic.java @@ -113,6 +113,13 @@ public List getFeedbackQuestionsForSession(FeedbackSession fee return questions; } + /** + * Gets the unique feedback question based on sessionId and questionNumber. + */ + public FeedbackQuestion getFeedbackQuestionForSessionQuestionNumber(UUID sessionId, int questionNumber) { + return fqDb.getFeedbackQuestionForSessionQuestionNumber(sessionId, questionNumber); + } + /** * Checks if there are any questions for the given session that instructors can view/submit. */ diff --git a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java index bd2b6b43072..d20038d5403 100644 --- a/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java +++ b/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java @@ -206,24 +206,14 @@ public FeedbackResponse updateFeedbackResponseCascade(FeedbackResponse feedbackR FeedbackResponse oldResponse = frDb.getFeedbackResponse(feedbackResponse.getId()); FeedbackResponse newResponse = frDb.updateFeedbackResponse(feedbackResponse); - boolean isGiverSectionChanged = !oldResponse.getGiverSection().equals(newResponse.getGiverSection()); - boolean isRecipientSectionChanged = !oldResponse.getRecipientSection().equals(newResponse.getRecipientSection()); - - if (isGiverSectionChanged || isRecipientSectionChanged) { - List oldResponseComments = - frcLogic.getFeedbackResponseCommentForResponse(oldResponse.getId()); - for (FeedbackResponseComment oldResponseComment : oldResponseComments) { - if (isGiverSectionChanged) { - oldResponseComment.setGiverSection(newResponse.getGiverSection()); - } + List oldResponseComments = + frcLogic.getFeedbackResponseCommentForResponse(oldResponse.getId()); - if (isRecipientSectionChanged) { - oldResponseComment.setRecipientSection(newResponse.getRecipientSection()); - } - - frcLogic.updateFeedbackResponseComment(oldResponseComment); - } + for (FeedbackResponseComment oldResponseComment : oldResponseComments) { + oldResponseComment.setGiverSection(newResponse.getGiverSection()); + oldResponseComment.setRecipientSection(newResponse.getRecipientSection()); + frcLogic.updateFeedbackResponseComment(oldResponseComment); } return newResponse; @@ -303,6 +293,18 @@ public List getFeedbackResponsesForRecipientForCourse( return frDb.getFeedbackResponsesForRecipientForCourse(courseId, recipient); } + /** + * Gets all responses from a specific giver and recipient for a course. + */ + public List getFeedbackResponsesFromGiverAndRecipientForCourse( + String courseId, String giverEmail, String recipientEmail) { + assert courseId != null; + assert giverEmail != null; + assert recipientEmail != null; + + return frDb.getFeedbackResponsesForGiverAndRecipientForCourse(courseId, giverEmail, recipientEmail); + } + /** * Gets all responses given by a user for a question. */ diff --git a/src/main/java/teammates/storage/sqlapi/FeedbackQuestionsDb.java b/src/main/java/teammates/storage/sqlapi/FeedbackQuestionsDb.java index ba4ac18b394..cee43672353 100644 --- a/src/main/java/teammates/storage/sqlapi/FeedbackQuestionsDb.java +++ b/src/main/java/teammates/storage/sqlapi/FeedbackQuestionsDb.java @@ -82,6 +82,22 @@ public List getFeedbackQuestionsForSession(UUID fdId) { return HibernateUtil.createQuery(cq).getResultList(); } + /** + * Gets the unique feedback question based on sessionId and questionNumber. + */ + public FeedbackQuestion getFeedbackQuestionForSessionQuestionNumber(UUID sessionId, int questionNumber) { + CriteriaBuilder cb = HibernateUtil.getCriteriaBuilder(); + CriteriaQuery cq = cb.createQuery(FeedbackQuestion.class); + Root fqRoot = cq.from(FeedbackQuestion.class); + Join fqJoin = fqRoot.join("feedbackSession"); + cq.select(fqRoot).where( + cb.and( + cb.equal(fqJoin.get("id"), sessionId), + cb.equal(fqRoot.get("questionNumber"), questionNumber) + )); + return HibernateUtil.createQuery(cq).getResultStream().findFirst().orElse(null); + } + /** * Gets a list of feedback questions by {@code feedbackSession} and {@code giverType}. * diff --git a/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java b/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java index 5dcf7ab375e..9afb50f7a45 100644 --- a/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java +++ b/src/main/java/teammates/storage/sqlapi/FeedbackResponsesDb.java @@ -90,6 +90,27 @@ public List getFeedbackResponsesForRecipientForCourse(String c return HibernateUtil.createQuery(cr).getResultList(); } + /** + * Gets all responses with a specific giver and recipient in a course. + */ + public List getFeedbackResponsesForGiverAndRecipientForCourse(String courseId, String giver, + String recipient) { + CriteriaBuilder cb = HibernateUtil.getCriteriaBuilder(); + CriteriaQuery cr = cb.createQuery(FeedbackResponse.class); + Root frRoot = cr.from(FeedbackResponse.class); + Join fqJoin = frRoot.join("feedbackQuestion"); + Join fsJoin = fqJoin.join("feedbackSession"); + Join cJoin = fsJoin.join("course"); + + cr.select(frRoot) + .where(cb.and( + cb.equal(cJoin.get("id"), courseId), + cb.equal(frRoot.get("recipient"), recipient), + cb.equal(frRoot.get("giver"), giver))); + + return HibernateUtil.createQuery(cr).getResultList(); + } + /** * Creates a feedbackResponse. */ diff --git a/src/main/java/teammates/storage/sqlentity/FeedbackSession.java b/src/main/java/teammates/storage/sqlentity/FeedbackSession.java index 78306541c18..d3084f30ee1 100644 --- a/src/main/java/teammates/storage/sqlentity/FeedbackSession.java +++ b/src/main/java/teammates/storage/sqlentity/FeedbackSession.java @@ -151,7 +151,7 @@ public FeedbackSession getCopy() { endTime, sessionVisibleFromTime, resultsVisibleFromTime, gracePeriod, isOpeningEmailEnabled, isClosingEmailEnabled, isPublishedEmailEnabled ); - + fs.setId(getId()); fs.setCreatedAt(getCreatedAt()); fs.setUpdatedAt(getUpdatedAt()); fs.setDeletedAt(getDeletedAt()); @@ -235,6 +235,10 @@ public void setCourse(Course course) { this.course = course; } + public String getCourseId() { + return course.getId(); + } + public String getName() { return name; } @@ -490,7 +494,7 @@ public boolean isWaitingToOpen() { public boolean isOpenedGivenExtendedDeadline(Instant extendedDeadline) { Instant now = Instant.now(); return (now.isAfter(startTime) || now.equals(startTime)) - && now.isBefore(extendedDeadline.plus(gracePeriod)) || now.isBefore(endTime.plus(gracePeriod)); + && (now.isBefore(extendedDeadline.plus(gracePeriod)) || now.isBefore(endTime.plus(gracePeriod))); } /** diff --git a/src/main/java/teammates/ui/webapi/BasicFeedbackSubmissionAction.java b/src/main/java/teammates/ui/webapi/BasicFeedbackSubmissionAction.java index 7c3b31931d5..f6997f58d54 100644 --- a/src/main/java/teammates/ui/webapi/BasicFeedbackSubmissionAction.java +++ b/src/main/java/teammates/ui/webapi/BasicFeedbackSubmissionAction.java @@ -1,5 +1,7 @@ package teammates.ui.webapi; +import java.time.Instant; + import teammates.common.datatransfer.FeedbackParticipantType; import teammates.common.datatransfer.attributes.FeedbackQuestionAttributes; import teammates.common.datatransfer.attributes.FeedbackSessionAttributes; @@ -12,6 +14,7 @@ import teammates.storage.sqlentity.Instructor; import teammates.storage.sqlentity.Section; import teammates.storage.sqlentity.Student; +import teammates.storage.sqlentity.User; /** * The basic action for feedback submission. @@ -407,10 +410,13 @@ void verifySessionOpenExceptForModeration(FeedbackSessionAttributes feedbackSess * *

If it is moderation request, omit the check. */ - void verifySessionOpenExceptForModeration(FeedbackSession feedbackSession) throws UnauthorizedAccessException { + void verifySessionOpenExceptForModeration(FeedbackSession feedbackSession, User user) + throws UnauthorizedAccessException { String moderatedPerson = getRequestParamValue(Const.ParamsNames.FEEDBACK_SESSION_MODERATED_PERSON); + Instant deadlineExtension = sqlLogic.getDeadlineForUser(feedbackSession, user); - if (StringHelper.isEmpty(moderatedPerson) && !(feedbackSession.isOpened() || feedbackSession.isInGracePeriod())) { + if (StringHelper.isEmpty(moderatedPerson) && !(feedbackSession.isOpenedGivenExtendedDeadline(deadlineExtension) + || feedbackSession.isInGracePeriodGivenExtendedDeadline(deadlineExtension))) { throw new UnauthorizedAccessException("The feedback session is not available for submission", true); } } diff --git a/src/main/java/teammates/ui/webapi/CreateFeedbackResponseCommentAction.java b/src/main/java/teammates/ui/webapi/CreateFeedbackResponseCommentAction.java index a7e4deeb41c..be68ba58550 100644 --- a/src/main/java/teammates/ui/webapi/CreateFeedbackResponseCommentAction.java +++ b/src/main/java/teammates/ui/webapi/CreateFeedbackResponseCommentAction.java @@ -87,7 +87,7 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException { session = session.getCopyForUser(student.getEmail()); gateKeeper.verifyAnswerableForStudent(feedbackQuestion); - verifySessionOpenExceptForModeration(session); + verifySessionOpenExceptForModeration(session, student); verifyInstructorCanSeeQuestionIfInModeration(feedbackQuestion); verifyNotPreview(); @@ -103,7 +103,7 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException { session = session.getCopyForUser(instructorAsFeedbackParticipant.getEmail()); gateKeeper.verifyAnswerableForInstructor(feedbackQuestion); - verifySessionOpenExceptForModeration(session); + verifySessionOpenExceptForModeration(session, instructorAsFeedbackParticipant); verifyInstructorCanSeeQuestionIfInModeration(feedbackQuestion); verifyNotPreview(); diff --git a/src/main/java/teammates/ui/webapi/DeleteFeedbackResponseCommentAction.java b/src/main/java/teammates/ui/webapi/DeleteFeedbackResponseCommentAction.java index 5ca18c8f252..c70eb9d2291 100644 --- a/src/main/java/teammates/ui/webapi/DeleteFeedbackResponseCommentAction.java +++ b/src/main/java/teammates/ui/webapi/DeleteFeedbackResponseCommentAction.java @@ -110,7 +110,7 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException { checkAccessControlForStudentFeedbackSubmission(student, session); session = session.getCopyForUser(student.getEmail()); - verifySessionOpenExceptForModeration(session); + verifySessionOpenExceptForModeration(session, student); gateKeeper.verifyOwnership(comment, question.getGiverType() == FeedbackParticipantType.TEAMS ? student.getTeamName() : student.getEmail()); @@ -124,7 +124,7 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException { checkAccessControlForInstructorFeedbackSubmission(instructorAsFeedbackParticipant, session); session = session.getCopyForUser(instructorAsFeedbackParticipant.getEmail()); - verifySessionOpenExceptForModeration(session); + verifySessionOpenExceptForModeration(session, instructorAsFeedbackParticipant); gateKeeper.verifyOwnership(comment, instructorAsFeedbackParticipant.getEmail()); break; case INSTRUCTOR_RESULT: diff --git a/src/main/java/teammates/ui/webapi/SubmitFeedbackResponsesAction.java b/src/main/java/teammates/ui/webapi/SubmitFeedbackResponsesAction.java index 9604462e50d..f384cb9a8f4 100644 --- a/src/main/java/teammates/ui/webapi/SubmitFeedbackResponsesAction.java +++ b/src/main/java/teammates/ui/webapi/SubmitFeedbackResponsesAction.java @@ -88,20 +88,20 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException { gateKeeper.verifyAnswerableForStudent(feedbackQuestion); Student student = getSqlStudentOfCourseFromRequest(feedbackQuestion.getCourseId()); if (student == null) { - throw new EntityNotFoundException("Student does not exist."); + throw new UnauthorizedAccessException("Trying to access system using a non-existent student entity"); } feedbackSession = feedbackSession.getCopyForUser(student.getEmail()); - verifySessionOpenExceptForModeration(feedbackSession); + verifySessionOpenExceptForModeration(feedbackSession, student); 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."); + throw new UnauthorizedAccessException("Trying to access system using a non-existent instructor entity"); } feedbackSession = feedbackSession.getCopyForUser(instructor.getEmail()); - verifySessionOpenExceptForModeration(feedbackSession); + verifySessionOpenExceptForModeration(feedbackSession, instructor); checkAccessControlForInstructorFeedbackSubmission(instructor, feedbackSession); break; case INSTRUCTOR_RESULT: diff --git a/src/main/java/teammates/ui/webapi/UpdateFeedbackResponseCommentAction.java b/src/main/java/teammates/ui/webapi/UpdateFeedbackResponseCommentAction.java index ac11fb65571..a5f5ba14143 100644 --- a/src/main/java/teammates/ui/webapi/UpdateFeedbackResponseCommentAction.java +++ b/src/main/java/teammates/ui/webapi/UpdateFeedbackResponseCommentAction.java @@ -128,7 +128,7 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException { session = session.getCopyForUser(student.getEmail()); gateKeeper.verifyAnswerableForStudent(question); - verifySessionOpenExceptForModeration(session); + verifySessionOpenExceptForModeration(session, student); verifyInstructorCanSeeQuestionIfInModeration(question); verifyNotPreview(); @@ -145,7 +145,7 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException { session = session.getCopyForUser(instructorAsFeedbackParticipant.getEmail()); gateKeeper.verifyAnswerableForInstructor(question); - verifySessionOpenExceptForModeration(session); + verifySessionOpenExceptForModeration(session, instructorAsFeedbackParticipant); verifyInstructorCanSeeQuestionIfInModeration(question); verifyNotPreview(); diff --git a/src/test/java/teammates/ui/webapi/SubmitFeedbackResponsesActionTest.java b/src/test/java/teammates/ui/webapi/SubmitFeedbackResponsesActionTest.java index 4725d0f0ff0..0254a69a884 100644 --- a/src/test/java/teammates/ui/webapi/SubmitFeedbackResponsesActionTest.java +++ b/src/test/java/teammates/ui/webapi/SubmitFeedbackResponsesActionTest.java @@ -418,7 +418,7 @@ public void testAccessControl_submissionIsNotOpen_shouldFail() throws Exception setStudentDeadline(session, student, 30); int questionNumber = 2; - String[] submissionParams = buildSubmissionParams(session, questionNumber, Intent.INSTRUCTOR_SUBMISSION); + String[] submissionParams = buildSubmissionParams(session, questionNumber, Intent.STUDENT_SUBMISSION); verifyCannotAccess(submissionParams); } @@ -542,7 +542,7 @@ public void testAccessControl_studentSubmissionLoggedInAsAdmin_shouldFail() thro } @Test - public void testAccessControl_studentSubmissionLoggedInAsAdminMasqueradeAsStudent_shouldFail() throws Exception { + public void testAccessControl_studentSubmissionLoggedInAsAdminMasqueradeAsStudent_shouldAllow() throws Exception { FeedbackSessionAttributes session = getSession("gracePeriodSession"); StudentAttributes student = getStudent("student1InCourse1"); setEndTime(session, 1);