Skip to content

Commit

Permalink
[#12048] Resolve merge conflicts (#12776)
Browse files Browse the repository at this point in the history 
* remove extra methods

* remove method

* remove extra check

* fix failing tests

* fix 2/3 tests

* merge in GetFeedbackSessionActionTest from pr12773
  • Loading branch information
@weiquu
weiquu committed Feb 24, 2024
1 parent 9c53527 commit b697768
Show file tree
Hide file tree
Showing 8 changed files with 71 additions and 138 deletions.
6 changes: 4 additions & 2 deletions src/it/java/teammates/it/ui/webapi/GetCoursesActionIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,10 @@ private void verifySameCourseData(CourseData actualCourse, Course expectedCourse
private void verifySameCourseDataStudent(CourseData actualCourse, Course expectedCourse) {
assertEquals(actualCourse.getCourseId(), expectedCourse.getId());
assertEquals(actualCourse.getCourseName(), expectedCourse.getName());
assertEquals(actualCourse.getCreationTimestamp(), 0);
assertEquals(actualCourse.getDeletionTimestamp(), 0);
assertEquals(actualCourse.getCreationTimestamp(), expectedCourse.getCreatedAt().toEpochMilli());
if (expectedCourse.getDeletedAt() != null) {
assertEquals(actualCourse.getDeletionTimestamp(), expectedCourse.getDeletedAt().toEpochMilli());
}
assertEquals(actualCourse.getTimeZone(), expectedCourse.getTimeZone());
}

Expand Down
18 changes: 0 additions & 18 deletions src/main/java/teammates/ui/webapi/BasicCommentSubmissionAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,24 +18,6 @@ abstract class BasicCommentSubmissionAction extends BasicFeedbackSubmissionActio

static final String FEEDBACK_RESPONSE_COMMENT_EMPTY = "Comment cannot be empty";

/**
* Validates the questionType of the corresponding question.
*/
void validQuestionForCommentInSubmission(FeedbackQuestionAttributes feedbackQuestion) {
if (!feedbackQuestion.getQuestionDetailsCopy().isFeedbackParticipantCommentsOnResponsesAllowed()) {
throw new InvalidHttpParameterException("Invalid question type for comment in submission");
}
}

/**
* Validates the questionType of the corresponding question.
*/
void validQuestionForCommentInSubmission(FeedbackQuestion feedbackQuestion) {
if (!feedbackQuestion.getQuestionDetailsCopy().isFeedbackParticipantCommentsOnResponsesAllowed()) {
throw new InvalidHttpParameterException("Invalid question type for comment in submission");
}
}

/**
* Validates comment doesn't exist of corresponding response.
*/
Expand Down
112 changes: 56 additions & 56 deletions src/main/java/teammates/ui/webapi/BasicFeedbackSubmissionAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,25 +129,6 @@ void checkAccessControlForStudentFeedbackSubmission(
}
}

/**
* Checks the access control for student feedback result.
*/
void checkAccessControlForStudentFeedbackResult(
StudentAttributes student, FeedbackSessionAttributes feedbackSession) throws UnauthorizedAccessException {
if (student == null) {
throw new UnauthorizedAccessException("Trying to access system using a non-existent student entity");
}

String previewAsPerson = getRequestParamValue(Const.ParamsNames.PREVIEWAS);

if (StringHelper.isEmpty(previewAsPerson)) {
gateKeeper.verifyAccessible(student, feedbackSession);
verifyMatchingGoogleId(student.getGoogleId());
} else {
checkAccessControlForPreview(feedbackSession, false);
}
}

/**
* Checks the access control for student feedback submission.
*/
Expand Down Expand Up @@ -185,6 +166,25 @@ void checkAccessControlForStudentFeedbackSubmission(Student student, FeedbackSes
}
}

/**
* Checks the access control for student feedback result.
*/
void checkAccessControlForStudentFeedbackResult(
StudentAttributes student, FeedbackSessionAttributes feedbackSession) throws UnauthorizedAccessException {
if (student == null) {
throw new UnauthorizedAccessException("Trying to access system using a non-existent student entity");
}

String previewAsPerson = getRequestParamValue(Const.ParamsNames.PREVIEWAS);

if (StringHelper.isEmpty(previewAsPerson)) {
gateKeeper.verifyAccessible(student, feedbackSession);
verifyMatchingGoogleId(student.getGoogleId());
} else {
checkAccessControlForPreview(feedbackSession, false);
}
}

/**
* Gets the instructor involved in the submission process.
*/
Expand Down Expand Up @@ -241,6 +241,43 @@ void checkAccessControlForInstructorFeedbackSubmission(
}
}

/**
* Checks the access control for instructor feedback submission.
*/
void checkAccessControlForInstructorFeedbackSubmission(
Instructor instructor, FeedbackSession feedbackSession) throws UnauthorizedAccessException {
if (instructor == null) {
throw new UnauthorizedAccessException("Trying to access system using a non-existent instructor entity");
}

String moderatedPerson = getRequestParamValue(Const.ParamsNames.FEEDBACK_SESSION_MODERATED_PERSON);
String previewAsPerson = getRequestParamValue(Const.ParamsNames.PREVIEWAS);

if (!StringHelper.isEmpty(moderatedPerson)) {
gateKeeper.verifyLoggedInUserPrivileges(userInfo);
gateKeeper.verifyAccessible(
sqlLogic.getInstructorByGoogleId(feedbackSession.getCourse().getId(), userInfo.getId()),
feedbackSession, Const.InstructorPermissions.CAN_MODIFY_SESSION_COMMENT_IN_SECTIONS);
} else if (!StringHelper.isEmpty(previewAsPerson)) {
gateKeeper.verifyLoggedInUserPrivileges(userInfo);
gateKeeper.verifyAccessible(
sqlLogic.getInstructorByGoogleId(feedbackSession.getCourse().getId(), userInfo.getId()),
feedbackSession, Const.InstructorPermissions.CAN_MODIFY_SESSION);
} else {
gateKeeper.verifySessionSubmissionPrivilegeForInstructor(feedbackSession, instructor);
if (instructor.getAccount() != null) {
if (userInfo == null) {
// Instructor is associated to an account; even if registration key is passed, do not allow access
throw new UnauthorizedAccessException("Login is required to access this feedback session");
} else if (!userInfo.id.equals(instructor.getAccount().getGoogleId())) {
// Logged in instructor is not the same as the instructor registered for the given key,
// do not allow access
throw new UnauthorizedAccessException("You are not authorized to access this feedback session");
}
}
}
}

/**
* Checks the access control for instructor feedback result.
*/
Expand Down Expand Up @@ -288,43 +325,6 @@ private void checkAccessControlForPreview(FeedbackSessionAttributes feedbackSess
}
}

/**
* Checks the access control for instructor feedback submission.
*/
void checkAccessControlForInstructorFeedbackSubmission(
Instructor instructor, FeedbackSession feedbackSession) throws UnauthorizedAccessException {
if (instructor == null) {
throw new UnauthorizedAccessException("Trying to access system using a non-existent instructor entity");
}

String moderatedPerson = getRequestParamValue(Const.ParamsNames.FEEDBACK_SESSION_MODERATED_PERSON);
String previewAsPerson = getRequestParamValue(Const.ParamsNames.PREVIEWAS);

if (!StringHelper.isEmpty(moderatedPerson)) {
gateKeeper.verifyLoggedInUserPrivileges(userInfo);
gateKeeper.verifyAccessible(
sqlLogic.getInstructorByGoogleId(feedbackSession.getCourse().getId(), userInfo.getId()),
feedbackSession, Const.InstructorPermissions.CAN_MODIFY_SESSION_COMMENT_IN_SECTIONS);
} else if (!StringHelper.isEmpty(previewAsPerson)) {
gateKeeper.verifyLoggedInUserPrivileges(userInfo);
gateKeeper.verifyAccessible(
sqlLogic.getInstructorByGoogleId(feedbackSession.getCourse().getId(), userInfo.getId()),
feedbackSession, Const.InstructorPermissions.CAN_MODIFY_SESSION);
} else {
gateKeeper.verifySessionSubmissionPrivilegeForInstructor(feedbackSession, instructor);
if (instructor.getAccount() != null) {
if (userInfo == null) {
// Instructor is associated to an account; even if registration key is passed, do not allow access
throw new UnauthorizedAccessException("Login is required to access this feedback session");
} else if (!userInfo.id.equals(instructor.getAccount().getGoogleId())) {
// Logged in instructor is not the same as the instructor registered for the given key,
// do not allow access
throw new UnauthorizedAccessException("You are not authorized to access this feedback session");
}
}
}
}

/**
* Verifies that it is not a preview request.
*/
Expand Down
2 changes: 0 additions & 2 deletions src/main/java/teammates/ui/webapi/CreateFeedbackResponseCommentAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException {

checkAccessControlForStudentFeedbackSubmission(student, session);

validQuestionForCommentInSubmission(feedbackQuestion);
verifyResponseOwnerShipForStudent(student, feedbackResponse, feedbackQuestion);
break;
case INSTRUCTOR_SUBMISSION:
Expand All @@ -110,7 +109,6 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException {

checkAccessControlForInstructorFeedbackSubmission(instructorAsFeedbackParticipant, session);

validQuestionForCommentInSubmission(feedbackQuestion);
verifyResponseOwnerShipForInstructor(instructorAsFeedbackParticipant, feedbackResponse);
break;
case INSTRUCTOR_RESULT:
Expand Down
32 changes: 8 additions & 24 deletions src/main/java/teammates/ui/webapi/SubmitFeedbackResponsesAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import teammates.common.datatransfer.attributes.FeedbackSessionAttributes;
import teammates.common.datatransfer.attributes.InstructorAttributes;
import teammates.common.datatransfer.attributes.StudentAttributes;
import teammates.common.datatransfer.questions.FeedbackQuestionType;
import teammates.common.datatransfer.questions.FeedbackResponseDetails;
import teammates.common.exception.EntityAlreadyExistsException;
import teammates.common.exception.EntityDoesNotExistException;
Expand Down Expand Up @@ -361,15 +360,6 @@ private JsonResult handleDataStoreExecute(FeedbackQuestionAttributes feedbackQue
FeedbackResponsesRequest submitRequest = getAndValidateRequestBody(FeedbackResponsesRequest.class);
log.info(JsonUtils.toCompactJson(submitRequest));

if (isSingleRecipientSubmission) {
// only keep the response for the recipient when the request is a single-recipient submission
List<FeedbackResponsesRequest.FeedbackResponseRequest> responseRequests = submitRequest.getResponses();
submitRequest.setResponses(
responseRequests.stream()
.filter(r -> recipientId.equals(r.getRecipient()))
.collect(Collectors.toList()));
}

for (String recipient : submitRequest.getRecipients()) {
if (!recipientsOfTheQuestion.containsKey(recipient)) {
throw new InvalidOperationException(
Expand Down Expand Up @@ -435,23 +425,17 @@ private JsonResult handleDataStoreExecute(FeedbackQuestionAttributes feedbackQue
.validateResponsesDetails(responseDetails, numRecipients);

if (!questionSpecificErrors.isEmpty()) {
throw new InvalidHttpRequestBodyException(String.join("\n", questionSpecificErrors));
throw new InvalidHttpRequestBodyException(questionSpecificErrors.toString());
}

if (!isSingleRecipientSubmission) {
List<String> recipients = submitRequest.getRecipients();
List<FeedbackResponseAttributes> feedbackResponsesToDelete = existingResponsesPerRecipient.entrySet().stream()
.filter(entry -> !recipients.contains(entry.getKey()))
.map(entry -> entry.getValue())
.collect(Collectors.toList());
List<String> recipients = submitRequest.getRecipients();
List<FeedbackResponseAttributes> feedbackResponsesToDelete = existingResponsesPerRecipient.entrySet().stream()
.filter(entry -> !recipients.contains(entry.getKey()))
.map(entry -> entry.getValue())
.collect(Collectors.toList());

for (FeedbackResponseAttributes feedbackResponse : feedbackResponsesToDelete) {
logic.deleteFeedbackResponseCascade(feedbackResponse.getId());
}
} else if (submitRequest.getRecipients().isEmpty() && existingResponsesPerRecipient.containsKey(recipientId)) {
// delete a single recipient submission
FeedbackResponseAttributes feedbackResponseToDelete = existingResponsesPerRecipient.get(recipientId);
logic.deleteFeedbackResponseCascade(feedbackResponseToDelete.getId());
for (FeedbackResponseAttributes feedbackResponse : feedbackResponsesToDelete) {
logic.deleteFeedbackResponseCascade(feedbackResponse.getId());
}

List<FeedbackResponseAttributes> output = new ArrayList<>();
Expand Down
3 changes: 1 addition & 2 deletions src/test/java/teammates/sqlui/webapi/GetCourseActionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,6 @@ void testExecute_asInstructor_success() {
@Test
void testExecute_asStudentHideCreatedAtAndDeletedAt_success() {
Course course = new Course("course-id", "name", Const.DEFAULT_TIME_ZONE, "institute");
course.setCreatedAt(Instant.parse("2022-01-01T00:00:00Z"));
course.setCreatedAt(Instant.parse("2023-01-01T00:00:00Z"));

when(mockLogic.getCourse(course.getId())).thenReturn(course);
Expand All @@ -181,7 +180,7 @@ void testExecute_asStudentHideCreatedAtAndDeletedAt_success() {
CourseData actionOutput = (CourseData) getJsonResult(getCourseAction).getOutput();

Course expectedCourse = course;
expectedCourse.setCreatedAt(Instant.ofEpochMilli(0));
expectedCourse.setCreatedAt(Instant.parse("2023-01-01T00:00:00Z"));
expectedCourse.setDeletedAt(Instant.ofEpochMilli(0));

assertEquals(JsonUtils.toJson(new CourseData(expectedCourse)), JsonUtils.toJson(actionOutput));
Expand Down
24 changes: 0 additions & 24 deletions src/test/java/teammates/ui/webapi/GetFeedbackResponsesActionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,30 +145,6 @@ protected void testExecute_commentSubmission_shouldGetCommentsSuccessfully() thr
verifyFeedbackCommentEquals(comment1FromStudent1, actualResponses.get(0).getGiverComment());
}

@Test
protected void testExecute_getTextQuestionType_shouldGetCommentsSuccessfully() throws Exception {
DataBundle dataBundle = loadDataBundle("/FeedbackResponseCommentCRUDTest.json");
removeAndRestoreDataBundle(dataBundle);

StudentAttributes student1InCourse1 = dataBundle.students.get("student1InCourse1");
FeedbackQuestionAttributes qn7InSession1 = dataBundle.feedbackQuestions.get("qn7InSession1");
FeedbackResponseAttributes response1ForQ7 = dataBundle.feedbackResponses.get("response1ForQ7");
FeedbackResponseCommentAttributes comment3FromStudent1 =
dataBundle.feedbackResponseComments.get("comment3FromStudent1");

loginAsStudent(student1InCourse1.getGoogleId());
String[] params = {
Const.ParamsNames.FEEDBACK_QUESTION_ID, qn7InSession1.getId(),
Const.ParamsNames.INTENT, Intent.STUDENT_SUBMISSION.toString(),
};
FeedbackResponsesData actualData = getFeedbackResponse(params);
List<FeedbackResponseData> actualResponses = actualData.getResponses();

assertEquals(1, actualResponses.size());
verifyFeedbackResponseEquals(response1ForQ7, actualResponses.get(0));
verifyFeedbackCommentEquals(comment3FromStudent1, actualResponses.get(0).getGiverComment());
}

@Test
@Override
protected void testAccessControl() {
Expand Down
12 changes: 2 additions & 10 deletions src/test/java/teammates/ui/webapi/GetFeedbackSessionActionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1257,7 +1257,7 @@ protected void testAccessControl_instructorResult() throws Exception {
______TS("Only instructor with correct privilege can access");

verifyOnlyInstructorsOfTheSameCourseWithCorrectCoursePrivilegeCanAccess(
Const.InstructorPermissions.CAN_VIEW_SESSION_IN_SECTIONS, params
Const.InstructorPermissions.CAN_SUBMIT_SESSION_IN_SECTIONS, params
);

______TS("Instructor moderates instructor submission with correct privilege will pass");
Expand All @@ -1268,21 +1268,13 @@ protected void testAccessControl_instructorResult() throws Exception {
verifyOnlyInstructorsOfTheSameCourseWithCorrectCoursePrivilegeCanAccess(
Const.InstructorPermissions.CAN_MODIFY_SESSION_COMMENT_IN_SECTIONS, params);

______TS("Instructor previews instructor submission with correct privilege will pass");
______TS("Instructor preview instructor result with correct privilege will pass");

String[] previewInstructorSubmissionParams =
generateParameters(feedbackSession, Intent.INSTRUCTOR_SUBMISSION,
"", "", instructor1OfCourse1.getEmail());
verifyOnlyInstructorsOfTheSameCourseWithCorrectCoursePrivilegeCanAccess(
Const.InstructorPermissions.CAN_MODIFY_SESSION, previewInstructorSubmissionParams);

______TS("Instructor previews instructor result with correct privilege will pass");

String[] previewInstructorResultParams =
generateParameters(feedbackSession, Intent.INSTRUCTOR_RESULT,
"", "", instructor1OfCourse1.getEmail());
verifyOnlyInstructorsOfTheSameCourseWithCorrectCoursePrivilegeCanAccess(
Const.InstructorPermissions.CAN_MODIFY_SESSION, previewInstructorResultParams);
}

private String[] generateParameters(FeedbackSessionAttributes session, Intent intent,
Expand Down

0 comments on commit b697768

Please sign in to comment.