diff --git a/src/e2e/java/teammates/e2e/cases/FeedbackResultsPageE2ETest.java b/src/e2e/java/teammates/e2e/cases/FeedbackResultsPageE2ETest.java index 72a50de1813..7dcc13d291d 100644 --- a/src/e2e/java/teammates/e2e/cases/FeedbackResultsPageE2ETest.java +++ b/src/e2e/java/teammates/e2e/cases/FeedbackResultsPageE2ETest.java @@ -58,7 +58,7 @@ public void testAll() { resultsPage.verifyFeedbackSessionDetails(openSession, course); ______TS("unregistered student: questions with responses loaded"); - verifyLoadedQuestions(unregistered); + verifyLoadedQuestions(unregistered, false); ______TS("registered student: can access results"); StudentAttributes student = testData.students.get("Alice"); @@ -70,7 +70,7 @@ public void testAll() { resultsPage.verifyFeedbackSessionDetails(openSession, course); ______TS("registered student: questions with responses loaded"); - verifyLoadedQuestions(student); + verifyLoadedQuestions(student, false); ______TS("verify responses"); questions.forEach(question -> verifyResponseDetails(student, question)); @@ -111,14 +111,57 @@ public void testAll() { resultsPage.verifyFeedbackSessionDetails(openSession, course); ______TS("registered instructor: questions with responses loaded"); - verifyLoadedQuestions(instructor); + verifyLoadedQuestions(instructor, false); ______TS("verify responses"); questions.forEach(question -> verifyResponseDetails(instructor, question)); + ______TS("preview results as student: can access results"); + url = createFrontendUrl(Const.WebPageURIs.SESSION_RESULTS_PAGE) + .withCourseId(openSession.getCourseId()) + .withSessionName(openSession.getFeedbackSessionName()) + .withParam("previewas", student.getEmail()); + resultsPage = getNewPageInstance(url, FeedbackResultsPage.class); + + resultsPage.verifyFeedbackSessionDetails(openSession, course); + + ______TS("preview results as student: questions with responses loaded and invisible responses excluded"); + verifyLoadedQuestions(student, true); + + ______TS("preview results as student: visible responses shown"); + questions.stream().filter(this::canInstructorSeeQuestion) + .forEach(question -> verifyResponseDetails(student, question)); + + ______TS("preview results as student: invisible comments excluded"); + List commentsNotVisibleForPreview = List.of( + testData.feedbackResponseComments.get("qn3Comment1").getCommentText()); + resultsPage.verifyQuestionHasCommentsNotVisibleForPreview(3, commentsNotVisibleForPreview); + + ______TS("preview results as student: visible comments shown"); + verifyCommentDetails(2, testData.feedbackResponseComments.get("qn2Comment1"), student); + verifyCommentDetails(2, testData.feedbackResponseComments.get("qn2Comment2"), student); + verifyCommentDetails(3, testData.feedbackResponseComments.get("qn3Comment2"), student); + verifyCommentDetails(4, testData.feedbackResponseComments.get("qn4Comment1"), student); + + ______TS("preview results as instructor: can access results"); + url = createFrontendUrl(Const.WebPageURIs.INSTRUCTOR_SESSION_RESULTS_PAGE) + .withCourseId(openSession.getCourseId()) + .withSessionName(openSession.getFeedbackSessionName()) + .withParam("previewas", instructor.getEmail()); + resultsPage = getNewPageInstance(url, FeedbackResultsPage.class); + + resultsPage.verifyFeedbackSessionDetails(openSession, course); + + ______TS("preview results as instructor: questions with responses loaded and invisible responses excluded"); + verifyLoadedQuestions(instructor, true); + + ______TS("preview results as instructor: visible responses shown"); + questions.stream().filter(this::canInstructorSeeQuestion) + .forEach(question -> verifyResponseDetails(instructor, question)); + } - private void verifyLoadedQuestions(StudentAttributes currentStudent) { + private void verifyLoadedQuestions(StudentAttributes currentStudent, boolean isPreview) { Set qnsWithResponse = getQnsWithResponses(currentStudent); questions.forEach(qn -> { if (qnsWithResponse.contains(qn)) { @@ -127,9 +170,17 @@ private void verifyLoadedQuestions(StudentAttributes currentStudent) { resultsPage.verifyQuestionNotPresent(qn.getQuestionNumber()); } }); + + if (isPreview) { + Set qnsWithResponseNotVisibleForPreview = qnsWithResponse.stream() + .filter(qn -> !canInstructorSeeQuestion(qn)) + .collect(Collectors.toSet()); + qnsWithResponseNotVisibleForPreview.forEach(qn -> + resultsPage.verifyQuestionHasResponsesNotVisibleForPreview(qn.getQuestionNumber())); + } } - private void verifyLoadedQuestions(InstructorAttributes currentInstructor) { + private void verifyLoadedQuestions(InstructorAttributes currentInstructor, boolean isPreview) { Set qnsWithResponse = getQnsWithResponses(currentInstructor); questions.forEach(qn -> { if (qnsWithResponse.contains(qn)) { @@ -138,6 +189,14 @@ private void verifyLoadedQuestions(InstructorAttributes currentInstructor) { resultsPage.verifyQuestionNotPresent(qn.getQuestionNumber()); } }); + + if (isPreview) { + Set qnsWithResponseNotVisibleForPreview = qnsWithResponse.stream() + .filter(qn -> !canInstructorSeeQuestion(qn)) + .collect(Collectors.toSet()); + qnsWithResponseNotVisibleForPreview.forEach(qn -> + resultsPage.verifyQuestionHasResponsesNotVisibleForPreview(qn.getQuestionNumber())); + } } private void verifyResponseDetails(StudentAttributes currentStudent, FeedbackQuestionAttributes question) { @@ -169,6 +228,16 @@ private void verifyCommentDetails(int questionNum, FeedbackResponseCommentAttrib resultsPage.verifyCommentDetails(questionNum, giver, editor, comment.getCommentText()); } + private boolean canInstructorSeeQuestion(FeedbackQuestionAttributes feedbackQuestion) { + boolean isGiverVisibleToInstructor = + feedbackQuestion.getShowGiverNameTo().contains(FeedbackParticipantType.INSTRUCTORS); + boolean isRecipientVisibleToInstructor = + feedbackQuestion.getShowRecipientNameTo().contains(FeedbackParticipantType.INSTRUCTORS); + boolean isResponseVisibleToInstructor = + feedbackQuestion.getShowResponsesTo().contains(FeedbackParticipantType.INSTRUCTORS); + return isResponseVisibleToInstructor && isGiverVisibleToInstructor && isRecipientVisibleToInstructor; + } + private Set getQnsWithResponses(StudentAttributes currentStudent) { return questions.stream() .filter(qn -> !getGivenResponses(currentStudent, qn).isEmpty() diff --git a/src/e2e/java/teammates/e2e/pageobjects/FeedbackResultsPage.java b/src/e2e/java/teammates/e2e/pageobjects/FeedbackResultsPage.java index 85f9cc043cb..c059def60b4 100644 --- a/src/e2e/java/teammates/e2e/pageobjects/FeedbackResultsPage.java +++ b/src/e2e/java/teammates/e2e/pageobjects/FeedbackResultsPage.java @@ -103,6 +103,16 @@ public void verifyQuestionNotPresent(int questionNum) { } } + public void verifyQuestionHasResponsesNotVisibleForPreview(int questionNum) { + verifyQuestionDoesNotShowResponses(questionNum); + verifyNonVisibleResponseAlertPresent(questionNum); + } + + public void verifyQuestionHasCommentsNotVisibleForPreview(int questionNum, List commentsNotVisible) { + verifyQuestionDoesNotShowComments(questionNum, commentsNotVisible); + verifyNonVisibleCommentAlertPresent(questionNum); + } + public void verifyNumScaleStatistics(int questionNum, String[] expectedStats) { verifyTableRowValues(getNumScaleStatistics(questionNum), expectedStats); } @@ -137,6 +147,47 @@ public void verifyCommentDetails(int questionNum, String commentGiver, String co } } + private void verifyQuestionDoesNotShowResponses(int questionNum) { + WebElement questionResponsesSection = getQuestionResponsesSection(questionNum); + try { + questionResponsesSection.findElement(By.className("all-responses")); + fail("Question " + questionNum + " should not display any actual responses when previewing results."); + } catch (NoSuchElementException e) { + // success + } + } + + private void verifyQuestionDoesNotShowComments(int questionNum, List commentsNotVisible) { + List commentsOfQuestion = getCommentFields(questionNum); + for (String commentString : commentsNotVisible) { + for (WebElement comment : commentsOfQuestion) { + if (comment.findElement(By.className("comment-text")).getText().equals(commentString)) { + fail("Comment \"" + commentString + "\" should not be present in question " + questionNum); + } + } + } + } + + private void verifyNonVisibleResponseAlertPresent(int questionNum) { + WebElement questionResponsesSection = getQuestionResponsesSection(questionNum); + try { + questionResponsesSection.findElement(By.className("non-visible-response-alert")); + } catch (NoSuchElementException e) { + fail("Question " + questionNum + + " should display an alert message for hidden responses when previewing results."); + } + } + + private void verifyNonVisibleCommentAlertPresent(int questionNum) { + WebElement questionResponsesSection = getQuestionResponsesSection(questionNum); + try { + questionResponsesSection.findElement(By.className("non-visible-comment-alert")); + } catch (NoSuchElementException e) { + fail("Question " + questionNum + + " should display an alert message for hidden comments when previewing results."); + } + } + private boolean hasDisplayedResponses(FeedbackQuestionAttributes question) { return !question.getQuestionDetailsCopy().getQuestionType().equals(FeedbackQuestionType.CONTRIB); } diff --git a/src/e2e/resources/data/FeedbackResultsPageE2ETest.json b/src/e2e/resources/data/FeedbackResultsPageE2ETest.json index f2e2f3ac38a..c49ff1fdb9d 100644 --- a/src/e2e/resources/data/FeedbackResultsPageE2ETest.json +++ b/src/e2e/resources/data/FeedbackResultsPageE2ETest.json @@ -301,15 +301,12 @@ "numberOfEntitiesToGiveFeedbackTo": 1, "showResponsesTo": [ "RECEIVER", - "OWN_TEAM_MEMBERS", - "INSTRUCTORS" + "OWN_TEAM_MEMBERS" ], "showGiverNameTo": [ - "INSTRUCTORS", "OWN_TEAM_MEMBERS" ], "showRecipientNameTo": [ - "INSTRUCTORS" ] }, "qn6": { @@ -345,8 +342,7 @@ "showRecipientNameTo": [ "RECEIVER", "OWN_TEAM_MEMBERS", - "STUDENTS", - "INSTRUCTORS" + "STUDENTS" ] }, "qn7": { @@ -425,7 +421,6 @@ "STUDENTS" ], "showGiverNameTo": [ - "INSTRUCTORS", "STUDENTS" ], "showRecipientNameTo": [ @@ -1158,10 +1153,12 @@ "receiverSection": "None", "feedbackResponseId": "2%Unregistered@gmail.tmt%FRes.alice.b@gmail.tmt", "showCommentTo": [ - "STUDENTS" + "STUDENTS", + "INSTRUCTORS" ], "showGiverNameTo": [ - "STUDENTS" + "STUDENTS", + "INSTRUCTORS" ], "commentGiverType": "INSTRUCTORS", "isVisibilityFollowingFeedbackQuestion": false, @@ -1178,10 +1175,12 @@ "receiverSection": "None", "feedbackResponseId": "2%Unregistered@gmail.tmt%FRes.alice.b@gmail.tmt", "showCommentTo": [ - "STUDENTS" + "STUDENTS", + "INSTRUCTORS" ], "showGiverNameTo": [ - "STUDENTS" + "STUDENTS", + "INSTRUCTORS" ], "commentGiverType": "INSTRUCTORS", "isVisibilityFollowingFeedbackQuestion": false, @@ -1215,8 +1214,12 @@ "giverSection": "None", "receiverSection": "None", "feedbackResponseId": "3%FRes.alice.b@gmail.tmt%FRes.benny.c@gmail.tmt", - "showCommentTo": [], - "showGiverNameTo": [], + "showCommentTo": [ + "INSTRUCTORS" + ], + "showGiverNameTo": [ + "INSTRUCTORS" + ], "commentGiverType": "STUDENTS", "isVisibilityFollowingFeedbackQuestion": true, "isCommentFromFeedbackParticipant": true, @@ -1231,8 +1234,12 @@ "giverSection": "None", "receiverSection": "None", "feedbackResponseId": "4%FRes.danny.e@gmail.tmt%%GENERAL%", - "showCommentTo": [], - "showGiverNameTo": [], + "showCommentTo": [ + "INSTRUCTORS" + ], + "showGiverNameTo": [ + "INSTRUCTORS" + ], "commentGiverType": "STUDENTS", "isVisibilityFollowingFeedbackQuestion": true, "isCommentFromFeedbackParticipant": true, diff --git a/src/main/java/teammates/common/datatransfer/SessionResultsBundle.java b/src/main/java/teammates/common/datatransfer/SessionResultsBundle.java index d50f000e373..6403edf66c6 100644 --- a/src/main/java/teammates/common/datatransfer/SessionResultsBundle.java +++ b/src/main/java/teammates/common/datatransfer/SessionResultsBundle.java @@ -4,6 +4,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import teammates.common.datatransfer.attributes.FeedbackQuestionAttributes; import teammates.common.datatransfer.attributes.FeedbackResponseAttributes; @@ -17,6 +18,8 @@ public class SessionResultsBundle { private final Map questionsMap; + private final Map questionsNotVisibleForPreviewMap; + private final Set questionsWithCommentNotVisibleForPreview; private final Map> questionResponseMap; private final Map> questionMissingResponseMap; private final Map> responseCommentsMap; @@ -26,6 +29,8 @@ public class SessionResultsBundle { private final CourseRoster roster; public SessionResultsBundle(Map questionsMap, + Map questionsNotVisibleForPreviewMap, + Set questionsWithCommentNotVisibleForPreview, List responses, List missingResponses, Map responseGiverVisibilityTable, @@ -35,6 +40,8 @@ public SessionResultsBundle(Map questionsMap CourseRoster roster) { this.questionsMap = questionsMap; + this.questionsNotVisibleForPreviewMap = questionsNotVisibleForPreviewMap; + this.questionsWithCommentNotVisibleForPreview = questionsWithCommentNotVisibleForPreview; this.responseCommentsMap = responseCommentsMap; this.responseGiverVisibilityTable = responseGiverVisibilityTable; this.responseRecipientVisibilityTable = responseRecipientVisibilityTable; @@ -137,6 +144,14 @@ public Map getQuestionsMap() { return questionsMap; } + public Map getQuestionsNotVisibleForPreviewMap() { + return questionsNotVisibleForPreviewMap; + } + + public Set getQuestionsWithCommentNotVisibleForPreview() { + return questionsWithCommentNotVisibleForPreview; + } + public Map> getResponseCommentsMap() { return responseCommentsMap; } diff --git a/src/main/java/teammates/logic/api/Logic.java b/src/main/java/teammates/logic/api/Logic.java index f08e1c9c42f..18f67c0db6e 100644 --- a/src/main/java/teammates/logic/api/Logic.java +++ b/src/main/java/teammates/logic/api/Logic.java @@ -1234,17 +1234,17 @@ public SessionResultsBundle getSessionResultsForCourse( /** * Gets the session result for a feedback session for the given user. * - * @see FeedbackResponsesLogic#getSessionResultsForUser(String, String, String, boolean, String) + * @see FeedbackResponsesLogic#getSessionResultsForUser(String, String, String, boolean, String, boolean) */ public SessionResultsBundle getSessionResultsForUser( String feedbackSessionName, String courseId, String userEmail, boolean isInstructor, - @Nullable String questionId) { + @Nullable String questionId, boolean isPreviewResults) { assert feedbackSessionName != null; assert courseId != null; assert userEmail != null; return feedbackResponsesLogic.getSessionResultsForUser( - feedbackSessionName, courseId, userEmail, isInstructor, questionId); + feedbackSessionName, courseId, userEmail, isInstructor, questionId, isPreviewResults); } /** diff --git a/src/main/java/teammates/logic/core/FeedbackResponsesLogic.java b/src/main/java/teammates/logic/core/FeedbackResponsesLogic.java index c64b4824261..45a91a6945c 100644 --- a/src/main/java/teammates/logic/core/FeedbackResponsesLogic.java +++ b/src/main/java/teammates/logic/core/FeedbackResponsesLogic.java @@ -344,10 +344,16 @@ private SessionResultsBundle buildResultsBundle( boolean isCourseWide, String feedbackSessionName, String courseId, String section, String questionId, boolean isInstructor, String userEmail, InstructorAttributes instructor, StudentAttributes student, CourseRoster roster, List allQuestions, - List allResponses) { + List allResponses, boolean isPreviewResults) { Map allQuestionsMap = new HashMap<>(); + Set questionsNotVisibleToInstructors = new HashSet<>(); for (FeedbackQuestionAttributes qn : allQuestions) { allQuestionsMap.put(qn.getId(), qn); + + // set questions that should not be visible to instructors if results are being previewed + if (isPreviewResults && !canInstructorsSeeQuestion(qn)) { + questionsNotVisibleToInstructors.add(qn.getId()); + } } // load comment(s) @@ -361,6 +367,8 @@ private SessionResultsBundle buildResultsBundle( // related questions, responses, and comment Map relatedQuestionsMap = new HashMap<>(); + Map relatedQuestionsNotVisibleForPreviewMap = new HashMap<>(); + Set relatedQuestionsWithCommentNotVisibleForPreview = new HashSet<>(); Map relatedResponsesMap = new HashMap<>(); Map> relatedCommentsMap = new HashMap<>(); if (isCourseWide) { @@ -385,6 +393,12 @@ private SessionResultsBundle buildResultsBundle( // build response for (FeedbackResponseAttributes response : allResponses) { + if (isPreviewResults + && relatedQuestionsNotVisibleForPreviewMap.get(response.getFeedbackQuestionId()) != null) { + // corresponding question's responses will not be shown to previewer, ignore the response + continue; + } + FeedbackQuestionAttributes correspondingQuestion = allQuestionsMap.get(response.getFeedbackQuestionId()); if (correspondingQuestion == null) { // orphan response without corresponding question, ignore it @@ -397,6 +411,13 @@ private SessionResultsBundle buildResultsBundle( continue; } + // if previewing results and corresponding question should not be visible to instructors, + // note down the question and do not add the response + if (isPreviewResults && questionsNotVisibleToInstructors.contains(response.getFeedbackQuestionId())) { + relatedQuestionsNotVisibleForPreviewMap.put(response.getFeedbackQuestionId(), correspondingQuestion); + continue; + } + // if there are viewable responses, the corresponding question becomes related relatedQuestionsMap.put(response.getFeedbackQuestionId(), correspondingQuestion); relatedResponsesMap.put(response.getId(), response); @@ -423,6 +444,13 @@ private SessionResultsBundle buildResultsBundle( continue; } + // if previewing results and the comment should not be visible to instructors, + // note down the corresponding question and do not add the comment + if (isPreviewResults && !canInstructorsSeeComment(frc)) { + relatedQuestionsWithCommentNotVisibleForPreview.add(frc.getFeedbackQuestionId()); + continue; + } + relatedCommentsMap.computeIfAbsent(relatedResponse.getId(), key -> new ArrayList<>()).add(frc); // generate comment giver name visibility table commentVisibilityTable.put(frc.getId(), frcLogic.isNameVisibleToUser(frc, relatedResponse, userEmail, roster)); @@ -438,9 +466,10 @@ private SessionResultsBundle buildResultsBundle( } RequestTracer.checkRemainingTime(); - return new SessionResultsBundle(relatedQuestionsMap, existingResponses, missingResponses, - responseGiverVisibilityTable, responseRecipientVisibilityTable, relatedCommentsMap, - commentVisibilityTable, roster); + return new SessionResultsBundle(relatedQuestionsMap, relatedQuestionsNotVisibleForPreviewMap, + relatedQuestionsWithCommentNotVisibleForPreview, + existingResponses, missingResponses, responseGiverVisibilityTable, responseRecipientVisibilityTable, + relatedCommentsMap, commentVisibilityTable, roster); } /** @@ -479,7 +508,7 @@ public SessionResultsBundle getSessionResultsForCourse( InstructorAttributes instructor = instructorsLogic.getInstructorForEmail(courseId, instructorEmail); return buildResultsBundle(true, feedbackSessionName, courseId, section, questionId, true, instructorEmail, - instructor, null, roster, allQuestions, allResponses); + instructor, null, roster, allQuestions, allResponses, false); } /** @@ -490,11 +519,12 @@ public SessionResultsBundle getSessionResultsForCourse( * @param userEmail the user viewing the feedback session * @param isInstructor true if the user is an instructor * @param questionId if not null, will only return partial bundle for the question + * @param isPreviewResults true if getting session results for preview purpose * @return the session result bundle */ public SessionResultsBundle getSessionResultsForUser( String feedbackSessionName, String courseId, String userEmail, boolean isInstructor, - @Nullable String questionId) { + @Nullable String questionId, boolean isPreviewResults) { CourseRoster roster = new CourseRoster( studentsLogic.getStudentsForCourse(courseId), instructorsLogic.getInstructorsForCourse(courseId)); @@ -518,7 +548,7 @@ public SessionResultsBundle getSessionResultsForUser( RequestTracer.checkRemainingTime(); return buildResultsBundle(false, feedbackSessionName, courseId, null, questionId, isInstructor, userEmail, - instructor, student, roster, allQuestions, allResponses); + instructor, student, roster, allQuestions, allResponses, isPreviewResults); } /** @@ -1101,6 +1131,30 @@ int getNumFeedbackResponsesByTimeRange(Instant startTime, Instant endTime) { return frDb.getNumFeedbackResponsesByTimeRange(startTime, endTime); } + /** + * Checks whether instructors can see the question. + */ + boolean canInstructorsSeeQuestion(FeedbackQuestionAttributes feedbackQuestion) { + boolean isResponseVisibleToInstructor = + feedbackQuestion.getShowResponsesTo().contains(FeedbackParticipantType.INSTRUCTORS); + boolean isGiverVisibleToInstructor = + feedbackQuestion.getShowGiverNameTo().contains(FeedbackParticipantType.INSTRUCTORS); + boolean isRecipientVisibleToInstructor = + feedbackQuestion.getShowRecipientNameTo().contains(FeedbackParticipantType.INSTRUCTORS); + return isResponseVisibleToInstructor && isGiverVisibleToInstructor && isRecipientVisibleToInstructor; + } + + /** + * Checks whether instructors can see the comment. + */ + boolean canInstructorsSeeComment(FeedbackResponseCommentAttributes feedbackResponseComment) { + boolean isCommentVisibleToInstructor = + feedbackResponseComment.getShowCommentTo().contains(FeedbackParticipantType.INSTRUCTORS); + boolean isGiverVisibleToInstructor = + feedbackResponseComment.getShowGiverNameTo().contains(FeedbackParticipantType.INSTRUCTORS); + return isCommentVisibleToInstructor && isGiverVisibleToInstructor; + } + /** * Set contains only unique response. */ diff --git a/src/main/java/teammates/ui/output/SessionResultsData.java b/src/main/java/teammates/ui/output/SessionResultsData.java index b55be5cf9a5..711b44a317b 100644 --- a/src/main/java/teammates/ui/output/SessionResultsData.java +++ b/src/main/java/teammates/ui/output/SessionResultsData.java @@ -49,7 +49,7 @@ public static SessionResultsData initForInstructor(SessionResultsBundle bundle) FeedbackQuestionAttributes question = bundle.getQuestionsMap().get(questionId); FeedbackQuestionDetails questionDetails = question.getQuestionDetailsCopy(); QuestionOutput qnOutput = new QuestionOutput(question, - questionDetails.getQuestionResultStatisticsJson(question, null, bundle)); + questionDetails.getQuestionResultStatisticsJson(question, null, bundle), false, false); // put normal responses List allResponses = buildResponsesForInstructor(responses, bundle, false); qnOutput.allResponses.addAll(allResponses); @@ -72,12 +72,15 @@ public static SessionResultsData initForStudent(SessionResultsBundle bundle, Stu Map> questionsWithResponses = bundle.getQuestionResponseMap(); - questionsWithResponses.forEach((questionId, responses) -> { FeedbackQuestionAttributes question = bundle.getQuestionsMap().get(questionId); FeedbackQuestionDetails questionDetails = question.getQuestionDetailsCopy(); + // check if question has comments (on any responses) not visible for preview + boolean hasCommentNotVisibleForPreview = bundle.getQuestionsWithCommentNotVisibleForPreview() + .contains(questionId); QuestionOutput qnOutput = new QuestionOutput(question, - questionDetails.getQuestionResultStatisticsJson(question, student.getEmail(), bundle)); + questionDetails.getQuestionResultStatisticsJson(question, student.getEmail(), bundle), + false, hasCommentNotVisibleForPreview); Map> otherResponsesMap = new HashMap<>(); qnOutput.getFeedbackQuestion().hideInformationForStudent(); @@ -117,6 +120,13 @@ public static SessionResultsData initForStudent(SessionResultsBundle bundle, Stu sessionResultsData.questions.add(qnOutput); }); + Map questionsWithResponsesNotVisibleForPreview = + bundle.getQuestionsNotVisibleForPreviewMap(); + questionsWithResponsesNotVisibleForPreview.forEach((questionId, question) -> { + QuestionOutput qnOutput = new QuestionOutput(question, "", true, false); + sessionResultsData.questions.add(qnOutput); + }); + return sessionResultsData; } @@ -375,15 +385,20 @@ public static class QuestionOutput { private final String questionStatistics; private final List allResponses = new ArrayList<>(); + private final boolean hasResponseButNotVisibleForPreview; + private final boolean hasCommentNotVisibleForPreview; // For student view only private final List responsesToSelf = new ArrayList<>(); private final List responsesFromSelf = new ArrayList<>(); private final List> otherResponses = new ArrayList<>(); - private QuestionOutput(FeedbackQuestionAttributes feedbackQuestionAttributes, String questionStatistics) { + private QuestionOutput(FeedbackQuestionAttributes feedbackQuestionAttributes, String questionStatistics, + boolean hasResponseButNotVisibleForPreview, boolean hasCommentNotVisibleForPreview) { this.feedbackQuestion = new FeedbackQuestionData(feedbackQuestionAttributes); this.questionStatistics = questionStatistics; + this.hasResponseButNotVisibleForPreview = hasResponseButNotVisibleForPreview; + this.hasCommentNotVisibleForPreview = hasCommentNotVisibleForPreview; } public FeedbackQuestionData getFeedbackQuestion() { @@ -398,6 +413,14 @@ public List getAllResponses() { return allResponses; } + public boolean getHasResponseButNotVisibleForPreview() { + return hasResponseButNotVisibleForPreview; + } + + public boolean getHasCommentNotVisibleForPreview() { + return hasCommentNotVisibleForPreview; + } + public List getResponsesFromSelf() { return responsesFromSelf; } diff --git a/src/main/java/teammates/ui/webapi/BasicFeedbackSubmissionAction.java b/src/main/java/teammates/ui/webapi/BasicFeedbackSubmissionAction.java index 9e205767680..b299393826e 100644 --- a/src/main/java/teammates/ui/webapi/BasicFeedbackSubmissionAction.java +++ b/src/main/java/teammates/ui/webapi/BasicFeedbackSubmissionAction.java @@ -74,21 +74,29 @@ void checkAccessControlForStudentFeedbackSubmission( student.getSection(), Const.InstructorPermissions.CAN_MODIFY_SESSION_COMMENT_IN_SECTIONS); } else if (!StringHelper.isEmpty(previewAsPerson)) { - gateKeeper.verifyLoggedInUserPrivileges(userInfo); - gateKeeper.verifyAccessible( - logic.getInstructorForGoogleId(feedbackSession.getCourseId(), userInfo.getId()), feedbackSession, - Const.InstructorPermissions.CAN_MODIFY_SESSION); + checkAccessControlForPreview(feedbackSession, false); } else { gateKeeper.verifyAccessible(student, feedbackSession); - if (!StringHelper.isEmpty(student.getGoogleId())) { - if (userInfo == null) { - // Student is associated to a google ID; 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(student.getGoogleId())) { - // Logged in student is not the same as the student registered for the given key, do not allow access - throw new UnauthorizedAccessException("You are not authorized to access this feedback session"); - } - } + verifyMatchingGoogleId(student.getGoogleId()); + } + } + + /** + * 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); } } @@ -125,24 +133,60 @@ void checkAccessControlForInstructorFeedbackSubmission( gateKeeper.verifyAccessible(logic.getInstructorForGoogleId(feedbackSession.getCourseId(), userInfo.getId()), feedbackSession, Const.InstructorPermissions.CAN_MODIFY_SESSION_COMMENT_IN_SECTIONS); } else if (!StringHelper.isEmpty(previewAsPerson)) { - gateKeeper.verifyLoggedInUserPrivileges(userInfo); - gateKeeper.verifyAccessible(logic.getInstructorForGoogleId(feedbackSession.getCourseId(), userInfo.getId()), - feedbackSession, Const.InstructorPermissions.CAN_MODIFY_SESSION); + checkAccessControlForPreview(feedbackSession, true); } else { gateKeeper.verifySessionSubmissionPrivilegeForInstructor(feedbackSession, instructor); - if (!StringHelper.isEmpty(instructor.getGoogleId())) { - if (userInfo == null) { - // Instructor is associated to a google ID; 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.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"); - } + verifyMatchingGoogleId(instructor.getGoogleId()); + } + } + + /** + * Checks the access control for instructor feedback result. + */ + void checkAccessControlForInstructorFeedbackResult( + InstructorAttributes instructor, FeedbackSessionAttributes feedbackSession) throws UnauthorizedAccessException { + if (instructor == null) { + throw new UnauthorizedAccessException("Trying to access system using a non-existent instructor entity"); + } + + String previewAsPerson = getRequestParamValue(Const.ParamsNames.PREVIEWAS); + + if (StringHelper.isEmpty(previewAsPerson)) { + gateKeeper.verifyAccessible(instructor, feedbackSession, + Const.InstructorPermissions.CAN_VIEW_SESSION_IN_SECTIONS); + verifyMatchingGoogleId(instructor.getGoogleId()); + } else { + checkAccessControlForPreview(feedbackSession, true); + } + } + + private void verifyMatchingGoogleId(String googleId) throws UnauthorizedAccessException { + if (!StringHelper.isEmpty(googleId)) { + if (userInfo == null) { + // Student/Instructor is associated to a google ID; 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(googleId)) { + // Logged in student/instructor is not the same as the student/instructor registered for the given key, + // do not allow access + throw new UnauthorizedAccessException("You are not authorized to access this feedback session"); } } } + private void checkAccessControlForPreview(FeedbackSessionAttributes feedbackSession, boolean isInstructor) + throws UnauthorizedAccessException { + gateKeeper.verifyLoggedInUserPrivileges(userInfo); + if (isInstructor) { + gateKeeper.verifyAccessible( + logic.getInstructorForGoogleId(feedbackSession.getCourseId(), userInfo.getId()), feedbackSession, + Const.InstructorPermissions.CAN_MODIFY_SESSION); + } else { + gateKeeper.verifyAccessible( + logic.getInstructorForGoogleId(feedbackSession.getCourseId(), userInfo.getId()), feedbackSession, + Const.InstructorPermissions.CAN_MODIFY_SESSION); + } + } + /** * Verifies that it is not a preview request. */ diff --git a/src/main/java/teammates/ui/webapi/GetFeedbackQuestionsAction.java b/src/main/java/teammates/ui/webapi/GetFeedbackQuestionsAction.java index 6630bd5b8c3..1c805f1f7dc 100644 --- a/src/main/java/teammates/ui/webapi/GetFeedbackQuestionsAction.java +++ b/src/main/java/teammates/ui/webapi/GetFeedbackQuestionsAction.java @@ -43,12 +43,12 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException { checkAccessControlForInstructorFeedbackSubmission(instructorAttributes, feedbackSession); break; case INSTRUCTOR_RESULT: - gateKeeper.verifyLoggedInUserPrivileges(userInfo); - gateKeeper.verifyAccessible(logic.getInstructorForGoogleId(courseId, userInfo.getId()), - feedbackSession, Const.InstructorPermissions.CAN_VIEW_SESSION_IN_SECTIONS); + instructorAttributes = getInstructorOfCourseFromRequest(courseId); + checkAccessControlForInstructorFeedbackResult(instructorAttributes, feedbackSession); break; case STUDENT_RESULT: - gateKeeper.verifyAccessible(getStudentOfCourseFromRequest(courseId), feedbackSession); + studentAttributes = getStudentOfCourseFromRequest(courseId); + checkAccessControlForStudentFeedbackResult(studentAttributes, feedbackSession); break; default: throw new InvalidHttpParameterException("Unknown intent " + intent); diff --git a/src/main/java/teammates/ui/webapi/GetFeedbackSessionAction.java b/src/main/java/teammates/ui/webapi/GetFeedbackSessionAction.java index 34a847dc9ca..fb13fa3b9e0 100644 --- a/src/main/java/teammates/ui/webapi/GetFeedbackSessionAction.java +++ b/src/main/java/teammates/ui/webapi/GetFeedbackSessionAction.java @@ -26,15 +26,21 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException { switch (intent) { case STUDENT_SUBMISSION: - case STUDENT_RESULT: StudentAttributes studentAttributes = getStudentOfCourseFromRequest(courseId); checkAccessControlForStudentFeedbackSubmission(studentAttributes, feedbackSession); break; + case STUDENT_RESULT: + studentAttributes = getStudentOfCourseFromRequest(courseId); + checkAccessControlForStudentFeedbackResult(studentAttributes, feedbackSession); + break; case INSTRUCTOR_SUBMISSION: - case INSTRUCTOR_RESULT: InstructorAttributes instructorAttributes = getInstructorOfCourseFromRequest(courseId); checkAccessControlForInstructorFeedbackSubmission(instructorAttributes, feedbackSession); break; + case INSTRUCTOR_RESULT: + instructorAttributes = getInstructorOfCourseFromRequest(courseId); + checkAccessControlForInstructorFeedbackResult(instructorAttributes, feedbackSession); + break; case FULL_DETAIL: gateKeeper.verifyLoggedInUserPrivileges(userInfo); gateKeeper.verifyAccessible(logic.getInstructorForGoogleId(courseId, userInfo.getId()), diff --git a/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java b/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java index 946dd44eba5..089e235768c 100644 --- a/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java +++ b/src/main/java/teammates/ui/webapi/GetSessionResultsAction.java @@ -6,13 +6,14 @@ import teammates.common.datatransfer.attributes.InstructorAttributes; import teammates.common.datatransfer.attributes.StudentAttributes; import teammates.common.util.Const; +import teammates.common.util.StringHelper; import teammates.ui.output.SessionResultsData; import teammates.ui.request.Intent; /** * Gets feedback session results including statistics where necessary. */ -class GetSessionResultsAction extends Action { +class GetSessionResultsAction extends BasicFeedbackSubmissionAction { @Override AuthType getMinAuthLevel() { @@ -26,6 +27,8 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException { FeedbackSessionAttributes fs = getNonNullFeedbackSession(feedbackSessionName, courseId); Intent intent = Intent.valueOf(getNonNullRequestParamValue(Const.ParamsNames.INTENT)); + String previewAsPerson = getRequestParamValue(Const.ParamsNames.PREVIEWAS); + boolean isPreviewResults = !StringHelper.isEmpty(previewAsPerson); switch (intent) { case FULL_DETAIL: gateKeeper.verifyLoggedInUserPrivileges(userInfo); @@ -33,18 +36,18 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException { gateKeeper.verifyAccessible(instructor, fs); break; case INSTRUCTOR_RESULT: - instructor = getPossiblyUnregisteredInstructor(courseId); - gateKeeper.verifyAccessible(instructor, fs); - if (!fs.isPublished()) { + if (!isPreviewResults && !fs.isPublished()) { throw new UnauthorizedAccessException("This feedback session is not yet published.", true); } + instructor = getInstructorOfCourseFromRequest(courseId); + checkAccessControlForInstructorFeedbackResult(instructor, fs); break; case STUDENT_RESULT: - StudentAttributes student = getPossiblyUnregisteredStudent(courseId); - gateKeeper.verifyAccessible(student, fs); - if (!fs.isPublished()) { + if (!isPreviewResults && !fs.isPublished()) { throw new UnauthorizedAccessException("This feedback session is not yet published.", true); } + StudentAttributes student = getStudentOfCourseFromRequest(courseId); + checkAccessControlForStudentFeedbackResult(student, fs); break; case INSTRUCTOR_SUBMISSION: case STUDENT_SUBMISSION: @@ -65,6 +68,9 @@ public JsonResult execute() { FeedbackResultFetchType fetchType = FeedbackResultFetchType.parseFetchType( getRequestParamValue(Const.ParamsNames.FEEDBACK_RESULTS_SECTION_BY_GIVER_RECEIVER)); + String previewAsPerson = getRequestParamValue(Const.ParamsNames.PREVIEWAS); + boolean isPreviewResults = !StringHelper.isEmpty(previewAsPerson); + SessionResultsBundle bundle; InstructorAttributes instructor; StudentAttributes student; @@ -78,10 +84,10 @@ public JsonResult execute() { return new JsonResult(SessionResultsData.initForInstructor(bundle)); case INSTRUCTOR_RESULT: // Section name filter is not applicable here - instructor = getPossiblyUnregisteredInstructor(courseId); + instructor = getInstructorOfCourseFromRequest(courseId); bundle = logic.getSessionResultsForUser(feedbackSessionName, courseId, instructor.getEmail(), - true, questionId); + true, questionId, isPreviewResults); // Build a fake student object, as the results will be displayed as if they are displayed to a student student = StudentAttributes.builder(instructor.getCourseId(), instructor.getEmail()) @@ -91,10 +97,10 @@ public JsonResult execute() { return new JsonResult(SessionResultsData.initForStudent(bundle, student)); case STUDENT_RESULT: // Section name filter is not applicable here - student = getPossiblyUnregisteredStudent(courseId); + student = getStudentOfCourseFromRequest(courseId); bundle = logic.getSessionResultsForUser(feedbackSessionName, courseId, student.getEmail(), - false, questionId); + false, questionId, isPreviewResults); return new JsonResult(SessionResultsData.initForStudent(bundle, student)); case INSTRUCTOR_SUBMISSION: diff --git a/src/test/java/teammates/common/datatransfer/SessionResultsBundleTest.java b/src/test/java/teammates/common/datatransfer/SessionResultsBundleTest.java index 48dcbbb0370..a69a8977d0d 100644 --- a/src/test/java/teammates/common/datatransfer/SessionResultsBundleTest.java +++ b/src/test/java/teammates/common/datatransfer/SessionResultsBundleTest.java @@ -2,6 +2,7 @@ import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; @@ -28,8 +29,9 @@ public void testGetQuestionResponseMap() { SessionResultsBundle bundle = new SessionResultsBundle( - responseBundle.feedbackQuestions, new ArrayList<>(responseBundle.feedbackResponses.values()), - new ArrayList<>(), new HashMap<>(), new HashMap<>(), new HashMap<>(), new HashMap<>(), + responseBundle.feedbackQuestions, new HashMap<>(), new HashSet<>(), + new ArrayList<>(responseBundle.feedbackResponses.values()), new ArrayList<>(), + new HashMap<>(), new HashMap<>(), new HashMap<>(), new HashMap<>(), new CourseRoster(new ArrayList<>(responseBundle.students.values()), new ArrayList<>(responseBundle.instructors.values()))); @@ -59,8 +61,8 @@ public void testGetQuestionMissingResponseMap() { SessionResultsBundle bundle = new SessionResultsBundle( - responseBundle.feedbackQuestions, new ArrayList<>(), - new ArrayList<>(responseBundle.feedbackResponses.values()), + responseBundle.feedbackQuestions, new HashMap<>(), new HashSet<>(), + new ArrayList<>(), new ArrayList<>(responseBundle.feedbackResponses.values()), new HashMap<>(), new HashMap<>(), new HashMap<>(), new HashMap<>(), new CourseRoster(new ArrayList<>(responseBundle.students.values()), new ArrayList<>(responseBundle.instructors.values()))); @@ -100,8 +102,9 @@ public void testIsResponseGiverRecipientVisible_typicalCase_shouldReturnCorrectV SessionResultsBundle bundle = new SessionResultsBundle( - responseBundle.feedbackQuestions, new ArrayList<>(responseBundle.feedbackResponses.values()), - new ArrayList<>(), responseGiverVisibilityTable, responseRecipientVisibilityTable, + responseBundle.feedbackQuestions, new HashMap<>(), new HashSet<>(), + new ArrayList<>(responseBundle.feedbackResponses.values()), new ArrayList<>(), + responseGiverVisibilityTable, responseRecipientVisibilityTable, new HashMap<>(), new HashMap<>(), new CourseRoster(new ArrayList<>(responseBundle.students.values()), new ArrayList<>(responseBundle.instructors.values()))); @@ -129,8 +132,9 @@ public void testIsCommentGiverVisible_typicalCase_shouldReturnCorrectValues() { SessionResultsBundle bundle = new SessionResultsBundle( - responseBundle.feedbackQuestions, new ArrayList<>(responseBundle.feedbackResponses.values()), - new ArrayList<>(), new HashMap<>(), new HashMap<>(), new HashMap<>(), commentGiverVisibilityTable, + responseBundle.feedbackQuestions, new HashMap<>(), new HashSet<>(), + new ArrayList<>(responseBundle.feedbackResponses.values()), new ArrayList<>(), + new HashMap<>(), new HashMap<>(), new HashMap<>(), commentGiverVisibilityTable, new CourseRoster(new ArrayList<>(responseBundle.students.values()), new ArrayList<>(responseBundle.instructors.values()))); diff --git a/src/test/java/teammates/common/datatransfer/questions/FeedbackContributionQuestionDetailsTest.java b/src/test/java/teammates/common/datatransfer/questions/FeedbackContributionQuestionDetailsTest.java index 97a91bf23ef..477312909a7 100644 --- a/src/test/java/teammates/common/datatransfer/questions/FeedbackContributionQuestionDetailsTest.java +++ b/src/test/java/teammates/common/datatransfer/questions/FeedbackContributionQuestionDetailsTest.java @@ -3,6 +3,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import org.testng.annotations.Test; @@ -119,8 +120,9 @@ public void testGetQuestionResultStatisticsJson() { SessionResultsBundle bundle = new SessionResultsBundle( - responseBundle.feedbackQuestions, new ArrayList<>(responseBundle.feedbackResponses.values()), - new ArrayList<>(), new HashMap<>(), new HashMap<>(), new HashMap<>(), new HashMap<>(), + responseBundle.feedbackQuestions, new HashMap<>(), new HashSet<>(), + new ArrayList<>(responseBundle.feedbackResponses.values()), new ArrayList<>(), + new HashMap<>(), new HashMap<>(), new HashMap<>(), new HashMap<>(), new CourseRoster(new ArrayList<>(responseBundle.students.values()), new ArrayList<>(responseBundle.instructors.values()))); diff --git a/src/test/java/teammates/logic/core/FeedbackResponsesLogicTest.java b/src/test/java/teammates/logic/core/FeedbackResponsesLogicTest.java index c289670de80..91acc39c1b1 100644 --- a/src/test/java/teammates/logic/core/FeedbackResponsesLogicTest.java +++ b/src/test/java/teammates/logic/core/FeedbackResponsesLogicTest.java @@ -1266,7 +1266,7 @@ public void testGetSessionResultsForUser_studentSpecificQuestion_shouldHaveCorre // Alice will see 3 responses SessionResultsBundle bundle = frLogic.getSessionResultsForUser( "First Session", "FQLogicPCT.CS2104", "FQLogicPCT.alice.b@gmail.tmt", - false, question.getId()); + false, question.getId(), false); assertEquals(1, bundle.getQuestionResponseMap().size()); List responseForQuestion = bundle.getQuestionResponseMap().entrySet().iterator().next().getValue(); @@ -1275,7 +1275,7 @@ public void testGetSessionResultsForUser_studentSpecificQuestion_shouldHaveCorre // Benny will see 3 responses bundle = frLogic.getSessionResultsForUser( "First Session", "FQLogicPCT.CS2104", "FQLogicPCT.benny.c@gmail.tmt", - false, question.getId()); + false, question.getId(), false); assertEquals(1, bundle.getQuestionResponseMap().size()); responseForQuestion = bundle.getQuestionResponseMap().entrySet().iterator().next().getValue(); assertEquals(3, responseForQuestion.size()); @@ -1283,7 +1283,7 @@ public void testGetSessionResultsForUser_studentSpecificQuestion_shouldHaveCorre // Charlie will see 2 responses bundle = frLogic.getSessionResultsForUser( "First Session", "FQLogicPCT.CS2104", "FQLogicPCT.charlie.d@gmail.tmt", - false, question.getId()); + false, question.getId(), false); assertEquals(1, bundle.getQuestionResponseMap().size()); responseForQuestion = bundle.getQuestionResponseMap().entrySet().iterator().next().getValue(); assertEquals(2, responseForQuestion.size()); @@ -1291,7 +1291,7 @@ public void testGetSessionResultsForUser_studentSpecificQuestion_shouldHaveCorre // Danny will see 2 responses bundle = frLogic.getSessionResultsForUser( "First Session", "FQLogicPCT.CS2104", "FQLogicPCT.danny.e@gmail.tmt", - false, question.getId()); + false, question.getId(), false); assertEquals(1, bundle.getQuestionResponseMap().size()); responseForQuestion = bundle.getQuestionResponseMap().entrySet().iterator().next().getValue(); assertEquals(2, responseForQuestion.size()); @@ -1299,7 +1299,7 @@ public void testGetSessionResultsForUser_studentSpecificQuestion_shouldHaveCorre // Emily will see 1 response bundle = frLogic.getSessionResultsForUser( "First Session", "FQLogicPCT.CS2104", "FQLogicPCT.emily.f@gmail.tmt", - false, question.getId()); + false, question.getId(), false); assertEquals(1, bundle.getQuestionResponseMap().size()); responseForQuestion = bundle.getQuestionResponseMap().entrySet().iterator().next().getValue(); assertEquals(1, responseForQuestion.size()); @@ -1314,9 +1314,12 @@ public void testGetSessionResultsForUser_studentAllQuestions_shouldGenerateCorre // Test result bundle for student1 StudentAttributes student = responseBundle.students.get("student1InCourse1"); + + ______TS("student himself/herself is viewing results"); + SessionResultsBundle bundle = frLogic.getSessionResultsForUser( session.getFeedbackSessionName(), session.getCourseId(), student.getEmail(), - false, null); + false, null, false); // Student can see responses: q1r1, q2r1,3, q3r1, qr4r2-3, q5r1, q7r1-2, q8r1-2 // We don't check the actual IDs as this is also implicitly tested @@ -1371,6 +1374,54 @@ public void testGetSessionResultsForUser_studentAllQuestions_shouldGenerateCorre // no entry in comment visibility table Map commentGiverVisibilityTable = bundle.getCommentGiverVisibilityTable(); assertEquals(0, commentGiverVisibilityTable.size()); + + // preview invisibility info should be empty because this is NOT previewing results + assertEquals(0, bundle.getQuestionsNotVisibleForPreviewMap().size()); + assertEquals(0, bundle.getQuestionsWithCommentNotVisibleForPreview().size()); + + ______TS("instructor is previewing results as student"); + + bundle = frLogic.getSessionResultsForUser( + session.getFeedbackSessionName(), session.getCourseId(), student.getEmail(), + false, null, true); + + // instructors can see responses (when previewing results as the student): qr4r2-3 + totalResponse = 0; + for (Map.Entry> entry + : bundle.getQuestionResponseMap().entrySet()) { + totalResponse += entry.getValue().size(); + } + totalMissingResponse = 0; + for (Map.Entry> entry + : bundle.getQuestionMissingResponseMap().entrySet()) { + totalMissingResponse += entry.getValue().size(); + } + assertEquals(2, totalResponse); + // student should not see missing responses + assertEquals(0, totalMissingResponse); + // only q4 and q6 are visible to instructors previewing the results + // but q6 has no viewable response for the student himself/herself + assertEquals(1, bundle.getQuestionsMap().size()); + assertEquals(1, bundle.getQuestionResponseMap().size()); + assertEquals(1, bundle.getQuestionMissingResponseMap().size()); + + // Test the generated response visibilityTable for userNames. + responseGiverVisibilityTable = bundle.getResponseGiverVisibilityTable(); + assertTrue(responseGiverVisibilityTable.get(getResponseId("qn4.resp2", responseBundle))); + assertFalse(responseGiverVisibilityTable.get(getResponseId("qn4.resp3", responseBundle))); + assertEquals(totalResponse, responseGiverVisibilityTable.size()); + + responseRecipientVisibilityTable = bundle.getResponseRecipientVisibilityTable(); + assertTrue(responseRecipientVisibilityTable.get(getResponseId("qn4.resp2", responseBundle))); + assertTrue(responseRecipientVisibilityTable.get(getResponseId("qn4.resp3", responseBundle))); + assertEquals(totalResponse, responseRecipientVisibilityTable.size()); + + // no entry in comment visibility table + assertEquals(0, bundle.getCommentGiverVisibilityTable().size()); + + // q1-3, q5, q7-8 have responses viewable for the student but not instructors + assertEquals(6, bundle.getQuestionsNotVisibleForPreviewMap().size()); + assertEquals(0, bundle.getQuestionsWithCommentNotVisibleForPreview().size()); } @Test @@ -1384,7 +1435,7 @@ public void testGetSessionResultsForUser_instructor_shouldGenerateCorrectBundle( InstructorAttributes instructor = responseBundle.instructors.get("instructor1OfCourse1"); SessionResultsBundle bundle = frLogic.getSessionResultsForUser( session.getFeedbackSessionName(), session.getCourseId(), instructor.getEmail(), - true, null); + true, null, false); // Instructor can see responses: q3r1, q6r1 // We don't check the actual IDs as this is also implicitly tested @@ -1421,6 +1472,10 @@ public void testGetSessionResultsForUser_instructor_shouldGenerateCorrectBundle( // no entry in comment visibility table Map commentGiverVisibilityTable = bundle.getCommentGiverVisibilityTable(); assertEquals(0, commentGiverVisibilityTable.size()); + + // preview invisibility info should be empty because this is NOT previewing results + assertEquals(0, bundle.getQuestionsNotVisibleForPreviewMap().size()); + assertEquals(0, bundle.getQuestionsWithCommentNotVisibleForPreview().size()); } @Test @@ -1650,7 +1705,7 @@ public void testGetSessionResultsForUser_orphanResponseInDB_shouldStillHandleCor SessionResultsBundle bundle = frLogic.getSessionResultsForUser( fq.getFeedbackSessionName(), fq.getCourseId(), student.getEmail(), - false, fq.getId()); + false, fq.getId(), false); assertEquals(1, bundle.getQuestionResponseMap().size()); List responseForQuestion = bundle.getQuestionResponseMap().entrySet().iterator().next().getValue(); diff --git a/src/test/java/teammates/ui/webapi/GetFeedbackSessionActionTest.java b/src/test/java/teammates/ui/webapi/GetFeedbackSessionActionTest.java index 00cf7a4b4af..769a9d8809c 100644 --- a/src/test/java/teammates/ui/webapi/GetFeedbackSessionActionTest.java +++ b/src/test/java/teammates/ui/webapi/GetFeedbackSessionActionTest.java @@ -1232,6 +1232,12 @@ protected void testAccessControl_studentResult() throws Exception { ______TS("Instructor preview student session"); params = generateParameters(feedbackSession, intent, "", "", student1InCourse1.getEmail()); + verifyOnlyInstructorsOfTheSameCourseWithCorrectCoursePrivilegeCanAccess( + Const.InstructorPermissions.CAN_MODIFY_SESSION, params); + + ______TS("Instructor preview student result"); + params = generateParameters(feedbackSession, Intent.STUDENT_RESULT, "", "", student1InCourse1.getEmail()); + verifyOnlyInstructorsOfTheSameCourseWithCorrectCoursePrivilegeCanAccess( Const.InstructorPermissions.CAN_MODIFY_SESSION, params); } @@ -1251,7 +1257,7 @@ protected void testAccessControl_instructorResult() throws Exception { ______TS("Only instructor with correct privilege can access"); verifyOnlyInstructorsOfTheSameCourseWithCorrectCoursePrivilegeCanAccess( - Const.InstructorPermissions.CAN_SUBMIT_SESSION_IN_SECTIONS, params + Const.InstructorPermissions.CAN_VIEW_SESSION_IN_SECTIONS, params ); ______TS("Instructor moderates instructor submission with correct privilege will pass"); @@ -1262,13 +1268,21 @@ protected void testAccessControl_instructorResult() throws Exception { verifyOnlyInstructorsOfTheSameCourseWithCorrectCoursePrivilegeCanAccess( Const.InstructorPermissions.CAN_MODIFY_SESSION_COMMENT_IN_SECTIONS, params); - ______TS("Instructor preview instructor result with correct privilege will pass"); + ______TS("Instructor previews instructor submission 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, diff --git a/src/test/java/teammates/ui/webapi/GetSessionResultsActionTest.java b/src/test/java/teammates/ui/webapi/GetSessionResultsActionTest.java index e1122bb8f1f..855687d3710 100644 --- a/src/test/java/teammates/ui/webapi/GetSessionResultsActionTest.java +++ b/src/test/java/teammates/ui/webapi/GetSessionResultsActionTest.java @@ -92,9 +92,33 @@ protected void testExecute() { } } - ______TS("typical: student accesses results of his/her course"); + ______TS("typical: instructor preview session results as student"); StudentAttributes studentAttributes = typicalBundle.students.get("student1InCourse1"); + + submissionParams = new String[] { + Const.ParamsNames.FEEDBACK_SESSION_NAME, accessibleFeedbackSession.getFeedbackSessionName(), + Const.ParamsNames.COURSE_ID, accessibleFeedbackSession.getCourseId(), + Const.ParamsNames.INTENT, Intent.STUDENT_RESULT.name(), + Const.ParamsNames.PREVIEWAS, studentAttributes.getEmail(), + }; + + a = getAction(submissionParams); + r = getJsonResult(a); + + output = (SessionResultsData) r.getOutput(); + expectedResults = SessionResultsData.initForStudent( + logic.getSessionResultsForUser(accessibleFeedbackSession.getFeedbackSessionName(), + accessibleFeedbackSession.getCourseId(), + studentAttributes.getEmail(), + false, null, true), + studentAttributes); + + assertTrue(isSessionResultsDataEqual(expectedResults, output)); + + ______TS("typical: student accesses results of his/her course"); + + studentAttributes = typicalBundle.students.get("student1InCourse1"); loginAsStudent(studentAttributes.getGoogleId()); submissionParams = new String[] { @@ -111,7 +135,7 @@ protected void testExecute() { logic.getSessionResultsForUser(accessibleFeedbackSession.getFeedbackSessionName(), accessibleFeedbackSession.getCourseId(), studentAttributes.getEmail(), - false, null), + false, null, false), studentAttributes); assertTrue(isSessionResultsDataEqual(expectedResults, output)); @@ -193,8 +217,10 @@ private boolean isSessionResultsDataEqual(SessionResultsData self, SessionResult private boolean isQuestionOutputEqual(SessionResultsData.QuestionOutput self, SessionResultsData.QuestionOutput other) { - if (!JsonUtils.toJson(self.getFeedbackQuestion()).equals(JsonUtils.toJson(self.getFeedbackQuestion())) - || !self.getQuestionStatistics().equals(other.getQuestionStatistics())) { + if (!JsonUtils.toJson(self.getFeedbackQuestion()).equals(JsonUtils.toJson(other.getFeedbackQuestion())) + || !self.getQuestionStatistics().equals(other.getQuestionStatistics()) + || self.getHasResponseButNotVisibleForPreview() != other.getHasResponseButNotVisibleForPreview() + || self.getHasCommentNotVisibleForPreview() != other.getHasCommentNotVisibleForPreview()) { return false; } List thisResponses; @@ -223,24 +249,6 @@ private boolean isResponseOutputEqual(SessionResultsData.ResponseOutput self, && self.getResponseDetails().getJsonString().equals(other.getResponseDetails().getJsonString()); } - @Test - public void testAccessControl_withRegistrationKey_shouldPass() throws Exception { - CourseAttributes typicalCourse1 = typicalBundle.courses.get("typicalCourse1"); - FeedbackSessionAttributes feedbackSessionAttributes = typicalBundle.feedbackSessions.get("session1InCourse1"); - StudentAttributes student1 = typicalBundle.students.get("student1InCourse1"); - student1 = logic.getStudentForEmail(student1.getCourse(), student1.getEmail()); - - String[] submissionParams = new String[] { - Const.ParamsNames.COURSE_ID, typicalCourse1.getId(), - Const.ParamsNames.FEEDBACK_SESSION_NAME, feedbackSessionAttributes.getFeedbackSessionName(), - Const.ParamsNames.INTENT, Intent.STUDENT_RESULT.toString(), - Const.ParamsNames.REGKEY, student1.getKey(), - }; - - logic.publishFeedbackSession(feedbackSessionAttributes.getFeedbackSessionName(), typicalCourse1.getId()); - verifyAccessibleForUnregisteredUsers(submissionParams); - } - @Test public void testAccessControl_withoutCorrectAuthInfoAccessStudentResult_shouldFail() throws Exception { CourseAttributes typicalCourse1 = typicalBundle.courses.get("typicalCourse1"); diff --git a/src/web/app/components/preview-session-result-panel/preview-session-result-panel.component.html b/src/web/app/components/preview-session-result-panel/preview-session-result-panel.component.html new file mode 100644 index 00000000000..65c3dc1070f --- /dev/null +++ b/src/web/app/components/preview-session-result-panel/preview-session-result-panel.component.html @@ -0,0 +1,37 @@ +
+
+
+ Preview Results: +
+
+ + + + + + + +
+
+ + + + + + + +
+
+
diff --git a/src/web/app/components/preview-session-result-panel/preview-session-result-panel.component.scss b/src/web/app/components/preview-session-result-panel/preview-session-result-panel.component.scss new file mode 100644 index 00000000000..fb8f09dba4b --- /dev/null +++ b/src/web/app/components/preview-session-result-panel/preview-session-result-panel.component.scss @@ -0,0 +1,8 @@ +.background-color-light-blue { + background-color: #EAEFF5; +} + +.preview-select { + float: left; + width: 50%; +} diff --git a/src/web/app/components/preview-session-result-panel/preview-session-result-panel.component.spec.ts b/src/web/app/components/preview-session-result-panel/preview-session-result-panel.component.spec.ts new file mode 100644 index 00000000000..70873a7b332 --- /dev/null +++ b/src/web/app/components/preview-session-result-panel/preview-session-result-panel.component.spec.ts @@ -0,0 +1,36 @@ +import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; +import { FormsModule } from '@angular/forms'; +import { RouterTestingModule } from '@angular/router/testing'; +import { NgbTooltipModule } from '@ng-bootstrap/ng-bootstrap'; +import { TeammatesRouterModule } from '../teammates-router/teammates-router.module'; +import { PreviewSessionResultPanelComponent } from './preview-session-result-panel.component'; + +describe('PreviewSessionPanelComponent', () => { + let component: PreviewSessionResultPanelComponent; + let fixture: ComponentFixture; + + beforeEach(waitForAsync(() => { + TestBed.configureTestingModule({ + declarations: [ + PreviewSessionResultPanelComponent, + ], + imports: [ + FormsModule, + RouterTestingModule, + NgbTooltipModule, + TeammatesRouterModule, + ], + }) + .compileComponents(); + })); + + beforeEach(() => { + fixture = TestBed.createComponent(PreviewSessionResultPanelComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + it('should create', () => { + expect(component).toBeTruthy(); + }); +}); diff --git a/src/web/app/components/preview-session-result-panel/preview-session-result-panel.component.ts b/src/web/app/components/preview-session-result-panel/preview-session-result-panel.component.ts new file mode 100644 index 00000000000..caeec3fc3cf --- /dev/null +++ b/src/web/app/components/preview-session-result-panel/preview-session-result-panel.component.ts @@ -0,0 +1,38 @@ +import { Component, Input } from '@angular/core'; +import { + Instructor, + Student, +} from '../../../types/api-output'; + +/** + * Displaying the preview session panel. + */ +@Component({ + selector: 'tm-preview-session-result-panel', + templateUrl: './preview-session-result-panel.component.html', + styleUrls: ['./preview-session-result-panel.component.scss'], +}) +export class PreviewSessionResultPanelComponent { + + @Input() + courseId: string = ''; + + @Input() + feedbackSessionName: string = ''; + + @Input() + studentsOfCourse: Student[] = []; + + @Input() + emailOfStudentToPreview: string = ''; + + @Input() + instructorsOfCourse: Instructor[] = []; + + @Input() + emailOfInstructorToPreview: string = ''; + + @Input() + forDisplayOnly: boolean = false; + +} diff --git a/src/web/app/components/preview-session-result-panel/preview-session-result-panel.module.ts b/src/web/app/components/preview-session-result-panel/preview-session-result-panel.module.ts new file mode 100644 index 00000000000..39497666caa --- /dev/null +++ b/src/web/app/components/preview-session-result-panel/preview-session-result-panel.module.ts @@ -0,0 +1,25 @@ +import { CommonModule } from '@angular/common'; +import { NgModule } from '@angular/core'; +import { FormsModule } from '@angular/forms'; +import { NgbTooltipModule } from '@ng-bootstrap/ng-bootstrap'; +import { TeammatesRouterModule } from '../teammates-router/teammates-router.module'; +import { PreviewSessionResultPanelComponent } from './preview-session-result-panel.component'; + +/** + * Module for panel used to select respondent and preview session results as the person. + */ +@NgModule({ + imports: [ + CommonModule, + FormsModule, + NgbTooltipModule, + TeammatesRouterModule, + ], + declarations: [ + PreviewSessionResultPanelComponent, + ], + exports: [ + PreviewSessionResultPanelComponent, + ], +}) +export class PreviewSessionResultPanelModule { } diff --git a/src/web/app/components/question-response-panel/__snapshots__/question-response-panel.component.spec.ts.snap b/src/web/app/components/question-response-panel/__snapshots__/question-response-panel.component.spec.ts.snap index 96d4f2b23d7..3ce633dd2d2 100644 --- a/src/web/app/components/question-response-panel/__snapshots__/question-response-panel.component.spec.ts.snap +++ b/src/web/app/components/question-response-panel/__snapshots__/question-response-panel.component.spec.ts.snap @@ -6,6 +6,7 @@ exports[`QuestionResponsePanelComponent should snap with feedback session with q __ngContext__={[Function Number]} feedbackSessionsService={[Function FeedbackSessionsService]} intent={[Function String]} + previewAsPerson="" questions={[Function Array]} regKey="" session={[Function Object]} @@ -47,66 +48,70 @@ exports[`QuestionResponsePanelComponent should snap with feedback session with q
- - Other responses (to you): - - Responses are not visible to you. -
-
- - Your own responses (to others): - -
- -
+
+ + Other responses (to you): + + Responses are not visible to you. +
+
+ + Your own responses (to others): + +
+
- - - To: - - recipient1 - -
-
- + + To: + + recipient1 + + +
-
- - - - - - - -
- - From: - - giver1 -
- - -
+ + + + + + + + + +
+ + From: + + giver1 +
+ + +
+
-
- + +
@@ -148,113 +153,117 @@ exports[`QuestionResponsePanelComponent should snap with feedback session with q
- - Other responses (to you): - - Responses are not visible to you. -
-
- - Your own responses (to others): - -
- -
+
+ + Other responses (to you): + + Responses are not visible to you. +
+
+ + Your own responses (to others): + +
+
- - - To: - - recipient2 - -
-
- + + To: + + recipient2 + + +
-
- - - - - - - -
- - From: - - giver1 -
- - -
+ + + + + + + + + +
+ + From: + + giver1 +
+ + +
+
-
- -
-
- -
+ +
+
+
- - - To: - - recipient3 - -
-
- + + To: + + recipient3 + + +
-
- - - - - - - -
- - From: - - giver1 -
- - -
+ + + + + + + + + +
+ + From: + + giver1 +
+ + +
+
-
- + +
@@ -296,130 +305,134 @@ exports[`QuestionResponsePanelComponent should snap with feedback session with q
- - Other responses (to you): - - Responses are not visible to you. -
-
- - Your own responses (to others): - -
- -
+
+ + Other responses (to you): + + Responses are not visible to you. +
+
+ + Your own responses (to others): + +
+
- - - To: - - recipient3 - -
-
- + + To: + + recipient3 + + +
-
- - - - - - - - + + + + + + + + - - -
- - From: - - giver1 -
- - -
- -
-
    +
+ + From: + + giver1 +
+ + +
+ +
-
  • - -
    +
  • +
    - - comment-giver-1 commented at + + comment-giver-1 commented at + + + 17 Jan 1:09 PM + -
    • + + + +
    + + + + + +
    -
    -     + +
    @@ -435,6 +448,7 @@ exports[`QuestionResponsePanelComponent should snap with feedback session with q __ngContext__={[Function Number]} feedbackSessionsService={[Function FeedbackSessionsService]} intent={[Function String]} + previewAsPerson="" questions={[Function Array]} regKey="" session={[Function Object]} @@ -471,160 +485,164 @@ exports[`QuestionResponsePanelComponent should snap with feedback session with q
    - - Other responses (to you): - - Responses are not visible to you. -
    -
    - - Your own responses (to others): - -
    - -
    +
    + + Other responses (to you): + + Responses are not visible to you. +
    +
    + + Your own responses (to others): + +
    +
    - - - To: - - recipient1 - -
    -
    - + + To: + + recipient1 + + +
    -
    - - - - - - - -
    - - From: - - giver1 -
    - - -
    + + + + + + + + + +
    + + From: + + giver1 +
    + + +
    +
    -
    - -
    -
    - -
    + +
    +
    +
    - - - To: - - recipient2 - -
    -
    - + + To: + + recipient2 + + +
    -
    - - - - - - - -
    - - From: - - giver1 -
    - - -
    + + + + + + + + + +
    + + From: + + giver1 +
    + + +
    +
    -
    -     -
    -
    - -
    + +
    +
    +
    - - - To: - - recipient3 - -
    -
    - + + To: + + recipient3 + + +
    -
    - - - - - - - -
    - - From: - - giver1 -
    - - -
    + + + + + + + + + +
    + + From: + + giver1 +
    + + +
    +
    -
    - + +
    @@ -661,66 +679,417 @@ exports[`QuestionResponsePanelComponent should snap with feedback session with q
    +
    + + Other responses (to you): + + Responses are not visible to you. +
    +
    + + Your own responses (to others): + +
    + +
    +
    + + + To: + + team1 + +
    +
    + + + + + + + + + +
    + + From: + + giver1 +
    + + +
    +
    +
    +     +
    +
    +
    + + + + + +`; + +exports[`QuestionResponsePanelComponent should snap with questions and responses when previewing results 1`] = ` + +
    +
    + +
    +
    + + +

    + Question 1: +

    + + How well did team member perform? + + +     +     + +
    +
    +     +
    +
    + +
    +
    + + +

    + Question 3: +

    + + Rate your teammates proficiency + + +     +
    - - Your own responses (to others): - -
    - -
    + +
    + + Other responses (to you): + + Responses are not visible to you. +
    +
    + + Your own responses (to others): + +
    +
    - - - To: - - team1 - + + + To: + + recipient3 + +
    +
    + + + + + + + + + + + + +
    + + From: + + giver1 +
    + + +
    + +
    +
      +
    • + +
      +
      +
      +
      + + + comment-giver-1 commented at + + + 17 Jan 1:09 PM + + +
    +
    + +
    +
    +
    +
    +
    + +
    +
    + +
    +
    + + +

    + Question 4: +

    + + Do you have any feedback for the course? + +     +     +
    +
    + + + +
    +
    +
    +
    + + Your own responses (to others): + +
    +
    - + + + To: + + - + + +
    -
    - - - - - - - -
    - - From: - - giver1 -
    - - -
    + + + + + + + + + +
    + + From: + + giver1 +
    + + +
    + Yes +
    +     +     +
    +
    -
    - + +
    diff --git a/src/web/app/components/question-response-panel/question-response-panel.component.html b/src/web/app/components/question-response-panel/question-response-panel.component.html index d50dfc17d2a..7cb0b48faba 100644 --- a/src/web/app/components/question-response-panel/question-response-panel.component.html +++ b/src/web/app/components/question-response-panel/question-response-panel.component.html @@ -8,35 +8,45 @@
    -
    - -
    -

    Individual Responses are not configured to be shown for this question type.

    +
    + - - -
    - +
    + +
    +

    Individual Responses are not configured to be shown for this question type.

    -
    - Other responses (to you): No responses received yet. + + +
    + +
    +
    + Other responses (to you): No responses received yet. +
    +     +
    + +
    + Other responses (to you): Responses are not visible to you.
    +
    + Your own responses (to others): +
    + +
    +
    - -
    - Other responses (to you): Responses are not visible to you. + + -
    - Your own responses (to others): -
    - -
    -
    diff --git a/src/web/app/components/question-response-panel/question-response-panel.component.spec.ts b/src/web/app/components/question-response-panel/question-response-panel.component.spec.ts index 8e7555fba60..c3236a67089 100644 --- a/src/web/app/components/question-response-panel/question-response-panel.component.spec.ts +++ b/src/web/app/components/question-response-panel/question-response-panel.component.spec.ts @@ -120,6 +120,26 @@ describe('QuestionResponsePanelComponent', () => { customNumberOfEntitiesToGiveFeedbackTo: 0, }; + const testQuestion4: FeedbackQuestion = { + feedbackQuestionId: 'feedbackQuestion4', + questionNumber: 4, + questionBrief: 'Do you have any feedback for the course?', + questionDescription: '', + questionDetails: { + questionType: FeedbackQuestionType.TEXT, + questionText: 'Do you have any feedback for the course?', + shouldAllowRichText: true, + } as FeedbackTextQuestionDetails, + questionType: FeedbackQuestionType.TEXT, + giverType: FeedbackParticipantType.STUDENTS, + recipientType: FeedbackParticipantType.NONE, + numberOfEntitiesToGiveFeedbackToSetting: NumberOfEntitiesToGiveFeedbackToSetting.UNLIMITED, + showResponsesTo: [FeedbackVisibilityType.INSTRUCTORS, FeedbackVisibilityType.STUDENTS], + showGiverNameTo: [FeedbackVisibilityType.INSTRUCTORS], + showRecipientNameTo: [FeedbackVisibilityType.INSTRUCTORS], + customNumberOfEntitiesToGiveFeedbackTo: 0, + }; + const testQuestionAnonymousResponse1: FeedbackQuestion = { feedbackQuestionId: 'feedbackQuestionAnonymousResponse1', questionNumber: 1, @@ -174,6 +194,8 @@ describe('QuestionResponsePanelComponent', () => { isLoading: false, isLoaded: false, hasResponse: true, + hasResponseButNotVisibleForPreview: false, + hasCommentNotVisibleForPreview: false, }; let component: QuestionResponsePanelComponent; @@ -240,6 +262,8 @@ describe('QuestionResponsePanelComponent', () => { isLoading: false, isLoaded: true, hasResponse: true, + hasResponseButNotVisibleForPreview: false, + hasCommentNotVisibleForPreview: false, }, { feedbackQuestion: testQuestion2, @@ -295,6 +319,8 @@ describe('QuestionResponsePanelComponent', () => { isLoading: false, isLoaded: true, hasResponse: true, + hasResponseButNotVisibleForPreview: false, + hasCommentNotVisibleForPreview: false, }, { feedbackQuestion: testQuestion3, @@ -333,6 +359,8 @@ describe('QuestionResponsePanelComponent', () => { isLoading: false, isLoaded: true, hasResponse: true, + hasResponseButNotVisibleForPreview: false, + hasCommentNotVisibleForPreview: false, }, ]; @@ -439,6 +467,8 @@ describe('QuestionResponsePanelComponent', () => { isLoading: false, isLoaded: true, hasResponse: true, + hasResponseButNotVisibleForPreview: false, + hasCommentNotVisibleForPreview: false, }, { feedbackQuestion: testQuestionAnonymousResponse2, @@ -480,6 +510,99 @@ describe('QuestionResponsePanelComponent', () => { isLoading: false, isLoaded: true, hasResponse: true, + hasResponseButNotVisibleForPreview: false, + hasCommentNotVisibleForPreview: false, + }, + ]; + + fixture.detectChanges(); + expect(fixture).toMatchSnapshot(); + }); + + it('should snap with questions and responses when previewing results', () => { + component.session = testFeedbackSession; + component.questions = [ + { + feedbackQuestion: testQuestion1, + questionStatistics: '', + allResponses: [], + responsesToSelf: [], + responsesFromSelf: [], + otherResponses: [[]], + isLoading: false, + isLoaded: true, + hasResponse: true, + hasResponseButNotVisibleForPreview: true, + hasCommentNotVisibleForPreview: false, + }, + { + feedbackQuestion: testQuestion3, + questionStatistics: '', + allResponses: [], + responsesToSelf: [], + responsesFromSelf: [ + { + isMissingResponse: false, + responseId: 'resp-id-5', + giver: 'giver1', + giverTeam: 'team1', + giverSection: 'section1', + recipient: 'recipient3', + recipientTeam: 'team2', + recipientSection: 'section2', + responseDetails: { + answer: [1], + } as FeedbackRubricResponseDetails, + instructorComments: [ + { + commentGiver: 'comment-giver-1', + lastEditorEmail: 'comment@egeg.com', + feedbackResponseCommentId: 1, + commentText: 'this is a text', + createdAt: 1402775804, + lastEditedAt: 1402775804, + isVisibilityFollowingFeedbackQuestion: true, + showGiverNameTo: [], + showCommentTo: [], + }, + ], + }, + ], + otherResponses: [[]], + isLoading: false, + isLoaded: true, + hasResponse: true, + hasResponseButNotVisibleForPreview: false, + hasCommentNotVisibleForPreview: true, + }, + { + feedbackQuestion: testQuestion4, + questionStatistics: '', + allResponses: [], + responsesToSelf: [], + responsesFromSelf: [ + { + isMissingResponse: false, + responseId: 'resp-id-7', + giver: 'giver1', + giverTeam: 'team1', + giverSection: 'section1', + recipient: '-', + recipientTeam: 'None', + recipientSection: '-', + responseDetails: { + answer: 'Yes', + questionType: 'TEXT', + } as FeedbackTextResponseDetails, + instructorComments: [], + }, + ], + otherResponses: [[]], + isLoading: false, + isLoaded: true, + hasResponse: true, + hasResponseButNotVisibleForPreview: false, + hasCommentNotVisibleForPreview: false, }, ]; @@ -496,6 +619,8 @@ describe('QuestionResponsePanelComponent', () => { feedbackQuestion: testQuestion1, questionStatistics: '', allResponses: [], + hasResponseButNotVisibleForPreview: false, + hasCommentNotVisibleForPreview: false, responsesToSelf: [], responsesFromSelf: [], otherResponses: [], @@ -514,6 +639,7 @@ describe('QuestionResponsePanelComponent', () => { feedbackSessionName: 'First Session', questionId: testQuestion1.feedbackQuestionId, key: '', + previewAs: '', }); expect(testFeedbackQuestionModel.isLoading).toBe(false); expect(testFeedbackQuestionModel.isLoaded).toBe(true); diff --git a/src/web/app/components/question-response-panel/question-response-panel.component.ts b/src/web/app/components/question-response-panel/question-response-panel.component.ts index 9a5c06f0845..e1b3a9d6e4c 100644 --- a/src/web/app/components/question-response-panel/question-response-panel.component.ts +++ b/src/web/app/components/question-response-panel/question-response-panel.component.ts @@ -61,6 +61,9 @@ export class QuestionResponsePanelComponent { @Input() regKey: string = ''; + @Input() + previewAsPerson: string = ''; + canUserSeeResponses(question: FeedbackQuestionModel): boolean { const showResponsesTo: FeedbackVisibilityType[] = question.feedbackQuestion.showResponsesTo; if (this.intent === Intent.STUDENT_RESULT) { @@ -88,6 +91,7 @@ export class QuestionResponsePanelComponent { feedbackSessionName: this.session.feedbackSessionName, intent: this.intent, key: this.regKey, + previewAs: this.previewAsPerson, }).subscribe({ next: (sessionResults: SessionResults) => { const responses: QuestionOutput = sessionResults.questions[0]; @@ -98,6 +102,8 @@ export class QuestionResponsePanelComponent { question.questionStatistics = responses.questionStatistics; question.responsesFromSelf = responses.responsesFromSelf; question.responsesToSelf = responses.responsesToSelf; + question.hasResponseButNotVisibleForPreview = responses.hasResponseButNotVisibleForPreview; + question.hasCommentNotVisibleForPreview = responses.hasCommentNotVisibleForPreview; } else { question.hasResponse = false; if (question.errorMessage) { diff --git a/src/web/app/pages-help/instructor-help-page/instructor-help-sessions-section/instructor-help-sessions-data.ts b/src/web/app/pages-help/instructor-help-page/instructor-help-sessions-section/instructor-help-sessions-data.ts index cfa9b3ffed7..76d2ae090e1 100644 --- a/src/web/app/pages-help/instructor-help-page/instructor-help-sessions-section/instructor-help-sessions-data.ts +++ b/src/web/app/pages-help/instructor-help-page/instructor-help-sessions-section/instructor-help-sessions-data.ts @@ -320,6 +320,8 @@ export const EXAMPLE_GRQ_RESPONSES: Record = { isMissingResponse: false, }, ], + hasResponseButNotVisibleForPreview: false, + hasCommentNotVisibleForPreview: false, responsesToSelf: [], responsesFromSelf: [], otherResponses: [], @@ -415,4 +417,6 @@ export const EXAMPLE_QUESTIONS_WITH_RESPONSES: FeedbackQuestionModel[] = [{ isLoaded: true, isLoading: false, hasResponse: true, + hasResponseButNotVisibleForPreview: false, + hasCommentNotVisibleForPreview: false, }]; diff --git a/src/web/app/pages-instructor/instructor-session-result-page/instructor-session-result-page.component.html b/src/web/app/pages-instructor/instructor-session-result-page/instructor-session-result-page.component.html index 13c27269469..a63c7f3f4d2 100644 --- a/src/web/app/pages-instructor/instructor-session-result-page/instructor-session-result-page.component.html +++ b/src/web/app/pages-instructor/instructor-session-result-page/instructor-session-result-page.component.html @@ -115,6 +115,13 @@

    +
    + +
    + Download session results
    Show Statistics ({{ showStatistics ? "✓" : "✗" }})
    diff --git a/src/web/app/pages-instructor/instructor-session-result-page/instructor-session-result-page.component.spec.ts b/src/web/app/pages-instructor/instructor-session-result-page/instructor-session-result-page.component.spec.ts index cbe14b06c3e..5e0e2248f5f 100644 --- a/src/web/app/pages-instructor/instructor-session-result-page/instructor-session-result-page.component.spec.ts +++ b/src/web/app/pages-instructor/instructor-session-result-page/instructor-session-result-page.component.spec.ts @@ -9,6 +9,9 @@ import { CommentsToCommentTableModelPipe } from '../../components/comment-box/co import { LoadingRetryModule } from '../../components/loading-retry/loading-retry.module'; import { LoadingSpinnerModule } from '../../components/loading-spinner/loading-spinner.module'; import { PanelChevronModule } from '../../components/panel-chevron/panel-chevron.module'; +import { + PreviewSessionResultPanelModule, +} from '../../components/preview-session-result-panel/preview-session-result-panel.module'; import { GqrRqgViewResponsesModule, } from '../../components/question-responses/gqr-rqg-view-responses/gqr-rqg-view-responses.module'; @@ -67,6 +70,7 @@ describe('InstructorSessionResultPageComponent', () => { TeammatesRouterModule, ViewResultsPanelModule, SectionTypeDescriptionModule, + PreviewSessionResultPanelModule, ], providers: [ CommentsToCommentTableModelPipe, diff --git a/src/web/app/pages-instructor/instructor-session-result-page/instructor-session-result-page.component.ts b/src/web/app/pages-instructor/instructor-session-result-page/instructor-session-result-page.component.ts index 61ad8a3e569..8ea2f64d881 100644 --- a/src/web/app/pages-instructor/instructor-session-result-page/instructor-session-result-page.component.ts +++ b/src/web/app/pages-instructor/instructor-session-result-page/instructor-session-result-page.component.ts @@ -24,6 +24,7 @@ import { FeedbackSessionPublishStatus, FeedbackSessionSubmissionStatus, FeedbackSessionSubmittedGiverSet, Instructor, + Instructors, QuestionOutput, ResponseOutput, ResponseVisibleSetting, SessionResults, SessionVisibleSetting, @@ -115,6 +116,9 @@ export class InstructorSessionResultPageComponent extends InstructorCommentsComp hasNoResponseLoadingFailed: boolean = false; allStudentsInCourse: Student[] = []; + emailOfStudentToPreview: string = ''; + allInstructorsInCourse: Instructor[] = []; + emailOfInstructorToPreview: string = ''; FeedbackSessionPublishStatus: typeof FeedbackSessionPublishStatus = FeedbackSessionPublishStatus; isExpandAll: boolean = false; @@ -254,6 +258,21 @@ export class InstructorSessionResultPageComponent extends InstructorCommentsComp }).subscribe({ next: (allStudents: Students) => { this.allStudentsInCourse = allStudents.students; + + // sort the student list based on team name and student name + this.allStudentsInCourse.sort((a: Student, b: Student): number => { + const teamNameCompare: number = a.teamName.localeCompare(b.teamName); + if (teamNameCompare === 0) { + return a.name.localeCompare(b.name); + } + return teamNameCompare; + }); + + // select the first student + if (this.allStudentsInCourse.length >= 1) { + this.emailOfStudentToPreview = this.allStudentsInCourse[0].email; + } + this.loadNoResponseStudents(courseId, feedbackSessionName); }, error: (resp: ErrorMessageOutput) => { @@ -261,6 +280,29 @@ export class InstructorSessionResultPageComponent extends InstructorCommentsComp }, }); + // load all instructors in course + this.instructorService.loadInstructors({ + courseId: this.courseId, + intent: Intent.FULL_DETAIL, + }).subscribe({ + next: (instructors: Instructors) => { + this.allInstructorsInCourse = instructors.instructors; + + // sort the instructor list based on name + this.allInstructorsInCourse.sort((a: Instructor, b: Instructor): number => { + return a.name.localeCompare(b.name); + }); + + // select the first instructor + if (this.allInstructorsInCourse.length >= 1) { + this.emailOfInstructorToPreview = this.allInstructorsInCourse[0].email; + } + }, + error: (resp: ErrorMessageOutput) => { + this.statusMessageService.showErrorToast(resp.error.message); + }, + }); + // load current instructor name this.instructorService.getInstructor({ courseId, diff --git a/src/web/app/pages-instructor/instructor-session-result-page/instructor-session-result-page.module.ts b/src/web/app/pages-instructor/instructor-session-result-page/instructor-session-result-page.module.ts index 0ec6d7a7c0d..e88d215cddc 100644 --- a/src/web/app/pages-instructor/instructor-session-result-page/instructor-session-result-page.module.ts +++ b/src/web/app/pages-instructor/instructor-session-result-page/instructor-session-result-page.module.ts @@ -7,6 +7,9 @@ import { NgbCollapseModule, NgbTooltipModule } from '@ng-bootstrap/ng-bootstrap' import { AjaxLoadingModule } from '../../components/ajax-loading/ajax-loading.module'; import { LoadingRetryModule } from '../../components/loading-retry/loading-retry.module'; import { LoadingSpinnerModule } from '../../components/loading-spinner/loading-spinner.module'; +import { + PreviewSessionResultPanelModule, +} from '../../components/preview-session-result-panel/preview-session-result-panel.module'; import { TeammatesCommonModule } from '../../components/teammates-common/teammates-common.module'; import { TeammatesRouterModule } from '../../components/teammates-router/teammates-router.module'; import { ViewResultsPanelModule } from '../../components/view-results-panel/view-results-panel.module'; @@ -46,6 +49,7 @@ const routes: Routes = [ TeammatesRouterModule, ViewResultsPanelModule, SectionTypeDescriptionModule, + PreviewSessionResultPanelModule, ], }) export class InstructorSessionResultPageModule { } diff --git a/src/web/app/pages-session/session-result-page/__snapshots__/session-result-page.component.spec.ts.snap b/src/web/app/pages-session/session-result-page/__snapshots__/session-result-page.component.spec.ts.snap index 93cdbaa4534..79992c6389e 100644 --- a/src/web/app/pages-session/session-result-page/__snapshots__/session-result-page.component.spec.ts.snap +++ b/src/web/app/pages-session/session-result-page/__snapshots__/session-result-page.component.spec.ts.snap @@ -1,5 +1,152 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`SessionResultPageComponent should snap when previewing results 1`] = ` + +

    + Feedback Session Results +

    +
    +

    + Previewing Session Results as + + student + + Alice2 (alice2@tmt.tmt) + +

    +
    +
    +
    +
    + +
    + CS3281 +
    +
    +
    + +
    + Peer Feedback +
    +
    +
    + +
    +
    +
    +
    + +
    +
    +
    +
    +
    +
    +
    + +
    + + +
    +     +     +`; + exports[`SessionResultPageComponent should snap when session results failed to load 1`] = ` Feedback Session Results +
    +
    +

    Previewing Session Results as + student + instructor + {{ personName }} ({{ previewAsPerson }}) + + +

    +
    +

    + The page below resembles the session results page as seen by {{ personName }} ({{ previewAsPerson }}). +

    +

    + Note that due to visibility settings, contents that are not supposed to be shown to instructors (e.g. yourself) but can be seen by the respondent will not be shown in the page below. + Instead, you will see a message box (not seen by the actual respondent) in their place. +

    +
    +
    +
    diff --git a/src/web/app/pages-session/session-result-page/session-result-page.component.scss b/src/web/app/pages-session/session-result-page/session-result-page.component.scss index 2ac942bbfd6..50e73706a5b 100644 --- a/src/web/app/pages-session/session-result-page/session-result-page.component.scss +++ b/src/web/app/pages-session/session-result-page/session-result-page.component.scss @@ -8,3 +8,7 @@ label { font-weight: bold; } + +.no-shadow { + box-shadow: none; +} diff --git a/src/web/app/pages-session/session-result-page/session-result-page.component.spec.ts b/src/web/app/pages-session/session-result-page/session-result-page.component.spec.ts index 6ba23b14501..9cffde1612e 100644 --- a/src/web/app/pages-session/session-result-page/session-result-page.component.spec.ts +++ b/src/web/app/pages-session/session-result-page/session-result-page.component.spec.ts @@ -111,6 +111,7 @@ describe('SessionResultPageComponent', () => { courseid: 'CS3281', fsname: 'Peer Feedback', key: 'reg-key', + previewas: '', }; beforeEach(waitForAsync(() => { @@ -240,6 +241,18 @@ describe('SessionResultPageComponent', () => { expect(fixture).toMatchSnapshot(); }); + it('should snap when previewing results', () => { + component.intent = Intent.STUDENT_RESULT; + component.regKey = ''; + component.previewAsPerson = 'alice2@tmt.tmt'; + component.personName = 'Alice2'; + component.personEmail = 'alice2@tmt.tmt'; + component.session = testFeedbackSession; + component.questions = []; + fixture.detectChanges(); + expect(fixture).toMatchSnapshot(); + }); + it('should fetch auth info on init', () => { jest.spyOn(authService, 'getAuthUser').mockReturnValue(of(testInfo)); @@ -379,6 +392,8 @@ describe('SessionResultPageComponent', () => { isLoading: false, isLoaded: false, hasResponse: true, + hasResponseButNotVisibleForPreview: false, + hasCommentNotVisibleForPreview: false, }; jest.spyOn(authService, 'getAuthUser').mockReturnValue(of(testInfo)); @@ -393,6 +408,7 @@ describe('SessionResultPageComponent', () => { feedbackSessionName: testQueryParams['fsname'], intent: Intent.STUDENT_RESULT, key: testQueryParams['key'], + previewAs: testQueryParams['previewas'], }); expect(component.questions.length).toEqual(1); expect(component.questions[0]).toEqual(testFeedbackQuestionModel); diff --git a/src/web/app/pages-session/session-result-page/session-result-page.component.ts b/src/web/app/pages-session/session-result-page/session-result-page.component.ts index 3f54d77ad4e..4ad7fd4566b 100644 --- a/src/web/app/pages-session/session-result-page/session-result-page.component.ts +++ b/src/web/app/pages-session/session-result-page/session-result-page.component.ts @@ -43,6 +43,8 @@ export interface FeedbackQuestionModel { isLoaded: boolean; hasResponse: boolean; errorMessage?: string; + hasResponseButNotVisibleForPreview: boolean; + hasCommentNotVisibleForPreview: boolean; } /** @@ -92,6 +94,9 @@ export class SessionResultPageComponent implements OnInit { intent: Intent = Intent.STUDENT_RESULT; + previewAsPerson: string = ''; + isPreviewHintExpanded: boolean = false; + isCourseLoading: boolean = true; isFeedbackSessionDetailsLoading: boolean = true; isFeedbackSessionResultsLoading: boolean = true; @@ -125,6 +130,7 @@ export class SessionResultPageComponent implements OnInit { this.courseId = queryParams.courseid; this.feedbackSessionName = queryParams.fsname; this.regKey = queryParams.key || ''; + this.previewAsPerson = queryParams.previewas ? queryParams.previewas : ''; if (queryParams.entitytype === 'instructor') { this.entityType = 'instructor'; this.intent = Intent.INSTRUCTOR_RESULT; @@ -133,9 +139,16 @@ export class SessionResultPageComponent implements OnInit { const nextUrl: string = `${window.location.pathname}${window.location.search.replace(/&/g, '%26')}`; this.authService.getAuthUser(undefined, nextUrl).subscribe({ next: (auth: AuthInfo) => { + const isPreview: boolean = !!(auth.user && this.previewAsPerson); if (auth.user) { this.loggedInUser = auth.user.id; } + // prevent having both key and previewas parameters in URL + if (this.regKey && isPreview) { + this.navigationService.navigateWithErrorMessage('/web/front', + 'You are not authorized to view this page.'); + return; + } if (this.regKey) { this.authService.getAuthRegkeyValidity(this.regKey, this.intent).subscribe({ next: (resp: RegkeyValidity) => { @@ -184,6 +197,7 @@ export class SessionResultPageComponent implements OnInit { }); } else if (this.loggedInUser) { // Load information based on logged in user + // This will also cover preview cases this.loadCourseInfo(); this.loadPersonName(); this.loadFeedbackSession(); @@ -205,7 +219,11 @@ export class SessionResultPageComponent implements OnInit { let request: Observable; switch (this.intent) { case Intent.STUDENT_RESULT: - request = this.courseService.getCourseAsStudent(this.courseId, this.regKey); + if (this.previewAsPerson) { + request = this.courseService.getCourseAsInstructor(this.courseId); + } else { + request = this.courseService.getCourseAsStudent(this.courseId, this.regKey); + } break; case Intent.INSTRUCTOR_RESULT: request = this.courseService.getCourseAsInstructor(this.courseId, this.regKey); @@ -229,7 +247,11 @@ export class SessionResultPageComponent implements OnInit { private loadPersonName(): void { switch (this.intent) { case Intent.STUDENT_RESULT: - this.studentService.getStudent(this.courseId, '', this.regKey).subscribe((student: Student) => { + this.studentService.getStudent( + this.courseId, + this.previewAsPerson, + this.regKey, + ).subscribe((student: Student) => { this.personName = student.name; this.personEmail = student.email; @@ -252,6 +274,7 @@ export class SessionResultPageComponent implements OnInit { feedbackSessionName: this.feedbackSessionName, intent: this.intent, key: this.regKey, + previewAs: this.previewAsPerson, }).subscribe((instructor: Instructor) => { this.personName = instructor.name; this.personEmail = instructor.email; @@ -269,6 +292,7 @@ export class SessionResultPageComponent implements OnInit { feedbackSessionName: this.feedbackSessionName, intent: this.intent, key: this.regKey, + previewAs: this.previewAsPerson, }) .pipe(finalize(() => { this.isFeedbackSessionDetailsLoading = false; })) .subscribe({ @@ -284,6 +308,7 @@ export class SessionResultPageComponent implements OnInit { feedbackSessionName: this.feedbackSessionName, intent: this.intent, key: this.regKey, + previewAs: this.previewAsPerson, }).pipe(finalize(() => { this.isFeedbackSessionResultsLoading = false; })) @@ -303,6 +328,8 @@ export class SessionResultPageComponent implements OnInit { isLoading: false, isLoaded: false, hasResponse: true, + hasResponseButNotVisibleForPreview: false, + hasCommentNotVisibleForPreview: false, }); } }, diff --git a/src/web/services/feedback-sessions.service.ts b/src/web/services/feedback-sessions.service.ts index e0f95c2e7f8..f67eb48e809 100644 --- a/src/web/services/feedback-sessions.service.ts +++ b/src/web/services/feedback-sessions.service.ts @@ -373,6 +373,7 @@ export class FeedbackSessionsService { groupBySection?: string, key?: string, sectionByGiverReceiver?: string, + previewAs?: string, }): Observable { const paramMap: Record = { courseid: queryParams.courseId, @@ -396,6 +397,10 @@ export class FeedbackSessionsService { paramMap['sectionByGiverReceiver'] = queryParams.sectionByGiverReceiver; } + if (queryParams.previewAs) { + paramMap['previewas'] = queryParams.previewAs; + } + return this.httpRequestService.get(ResourceEndpoints.RESULT, paramMap); }