Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#4570] Instructor: support previewing results as a student #12351

Merged
merged 49 commits into from
Apr 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
afa34de
Draft preview session result panel on session result page
fans2619 Jun 9, 2022
218c8d3
Allow viewing results as the person being previewed
fans2619 Jun 10, 2022
c226513
Remove questions not visible to instructors
fans2619 Jun 10, 2022
34f59f7
Clean up code and update tests
fans2619 Jun 10, 2022
5846527
Fix code structure
fans2619 Jun 10, 2022
bea9f22
Merge branch 'master' into 4570-preview-results-as-student
fans2619 Jun 13, 2022
f1dbb2a
Mark questions having responses visible to respondent but not instruc…
fans2619 Jun 13, 2022
bd671fa
Give alert for responses with comments invisible to instructors
fans2619 Jun 14, 2022
f5f7665
General update
fans2619 Jun 14, 2022
dfb9ead
Rename isForInstructorToPreview to isPreviewResults
fans2619 Jun 14, 2022
e9cc819
Add a getter to remove unread field
fans2619 Jun 14, 2022
759b8a5
Rename getter
fans2619 Jun 14, 2022
b8744e5
Show one "comment hidden" message for entire question
fans2619 Jun 16, 2022
c21eb2d
Refine UI
fans2619 Jun 16, 2022
39bb0b8
Add a test for session result page
fans2619 Jun 17, 2022
45ad530
Fix a bug on session result page
fans2619 Jun 17, 2022
908a7f1
Merge branch 'master' into 4570-preview-results-as-student
fans2619 Jun 19, 2022
0eb00dd
Add a test in GetSessionResultsActionTest
fans2619 Jun 19, 2022
e22809e
Add a test in FeedbackResponsesLogicTest
fans2619 Jun 19, 2022
a4e8cd9
Fix code style
fans2619 Jun 19, 2022
5161fa4
Add E2E tests for FeedbackResultsPage
fans2619 Jun 19, 2022
8ea3853
Update snapshots
fans2619 Jun 19, 2022
a837442
Merge branch 'master' into 4570-preview-results-as-student
fans2619 Jul 12, 2022
0d6f0ef
Refine navigation in session-result-page.component.ts
fans2619 Jul 12, 2022
dc01436
Update snapshot
fans2619 Jul 12, 2022
41b9cab
Merge branch 'master' into 4570-preview-results-as-student
fans2619 Jul 22, 2022
cfd8a3d
Merge branch 'master' into 4570-preview-results-as-student
fans2619 Jul 26, 2022
7a69734
Add the missing access control check
fans2619 Jul 27, 2022
8bda1c3
THIS COMMIT SHOULD BE DROPPED LATER
fans2619 Jul 27, 2022
022c8bd
Resolve merge conflicts
fans2619 Dec 2, 2022
0115ba0
Adjust code as needed by lazy loading of responses
fans2619 Dec 4, 2022
73125b1
Merge branch 'master' of https://github.com/TEAMMATES/teammates into …
fans2619 Dec 6, 2022
b90de34
Merge branch 'master' of https://github.com/TEAMMATES/teammates into …
fans2619 Dec 6, 2022
8471d3d
Delete debug code
fans2619 Dec 6, 2022
a5b56f6
Refine access control checking
fans2619 Dec 6, 2022
f46e3de
Fix code format
fans2619 Dec 8, 2022
b4b0dfe
Comment out a permission checking
fans2619 Dec 8, 2022
640452b
Modify and add some backend component tests
fans2619 Dec 8, 2022
4336dfe
Revert an irrelevant change
fans2619 Dec 8, 2022
92efc54
Update tests and snapshots
fans2619 Dec 8, 2022
871439c
Fix a bug in question-response-panel.component.html
fans2619 Dec 8, 2022
9b74883
Add two snapshot tests
fans2619 Dec 8, 2022
fbfb830
Merge branch 'master' of https://github.com/TEAMMATES/teammates into …
fans2619 Dec 8, 2022
4633668
Merge branch 'master' into 4570-revamp
wkurniawan07 Apr 7, 2023
dfd6c1f
Fix front-end implementations to follow new practices
wkurniawan07 Apr 7, 2023
9a52b24
Revert unnecessary comments and formatting changes
wkurniawan07 Apr 7, 2023
11887a8
Tidy up access control methods
wkurniawan07 Apr 7, 2023
e49dd22
Fix UI changes for Bootstrap 5 + accessibility
wkurniawan07 Apr 7, 2023
02e7596
Merge branch 'master' into 4570-revamp
samuelfangjw Apr 8, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 74 additions & 5 deletions src/e2e/java/teammates/e2e/cases/FeedbackResultsPageE2ETest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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));
Expand Down Expand Up @@ -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<String> 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<FeedbackQuestionAttributes> qnsWithResponse = getQnsWithResponses(currentStudent);
questions.forEach(qn -> {
if (qnsWithResponse.contains(qn)) {
Expand All @@ -127,9 +170,17 @@ private void verifyLoadedQuestions(StudentAttributes currentStudent) {
resultsPage.verifyQuestionNotPresent(qn.getQuestionNumber());
}
});

if (isPreview) {
Set<FeedbackQuestionAttributes> 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<FeedbackQuestionAttributes> qnsWithResponse = getQnsWithResponses(currentInstructor);
questions.forEach(qn -> {
if (qnsWithResponse.contains(qn)) {
Expand All @@ -138,6 +189,14 @@ private void verifyLoadedQuestions(InstructorAttributes currentInstructor) {
resultsPage.verifyQuestionNotPresent(qn.getQuestionNumber());
}
});

if (isPreview) {
Set<FeedbackQuestionAttributes> qnsWithResponseNotVisibleForPreview = qnsWithResponse.stream()
.filter(qn -> !canInstructorSeeQuestion(qn))
.collect(Collectors.toSet());
qnsWithResponseNotVisibleForPreview.forEach(qn ->
resultsPage.verifyQuestionHasResponsesNotVisibleForPreview(qn.getQuestionNumber()));
}
}

private void verifyResponseDetails(StudentAttributes currentStudent, FeedbackQuestionAttributes question) {
Expand Down Expand Up @@ -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<FeedbackQuestionAttributes> getQnsWithResponses(StudentAttributes currentStudent) {
return questions.stream()
.filter(qn -> !getGivenResponses(currentStudent, qn).isEmpty()
Expand Down
51 changes: 51 additions & 0 deletions src/e2e/java/teammates/e2e/pageobjects/FeedbackResultsPage.java
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,16 @@ public void verifyQuestionNotPresent(int questionNum) {
}
}

public void verifyQuestionHasResponsesNotVisibleForPreview(int questionNum) {
verifyQuestionDoesNotShowResponses(questionNum);
verifyNonVisibleResponseAlertPresent(questionNum);
}

public void verifyQuestionHasCommentsNotVisibleForPreview(int questionNum, List<String> commentsNotVisible) {
verifyQuestionDoesNotShowComments(questionNum, commentsNotVisible);
verifyNonVisibleCommentAlertPresent(questionNum);
}

public void verifyNumScaleStatistics(int questionNum, String[] expectedStats) {
verifyTableRowValues(getNumScaleStatistics(questionNum), expectedStats);
}
Expand Down Expand Up @@ -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<String> commentsNotVisible) {
List<WebElement> 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);
}
Expand Down
37 changes: 22 additions & 15 deletions src/e2e/resources/data/FeedbackResultsPageE2ETest.json
Original file line number Diff line number Diff line change
Expand Up @@ -301,15 +301,12 @@
"numberOfEntitiesToGiveFeedbackTo": 1,
"showResponsesTo": [
"RECEIVER",
"OWN_TEAM_MEMBERS",
"INSTRUCTORS"
"OWN_TEAM_MEMBERS"
],
"showGiverNameTo": [
"INSTRUCTORS",
"OWN_TEAM_MEMBERS"
],
"showRecipientNameTo": [
"INSTRUCTORS"
]
},
"qn6": {
Expand Down Expand Up @@ -345,8 +342,7 @@
"showRecipientNameTo": [
"RECEIVER",
"OWN_TEAM_MEMBERS",
"STUDENTS",
"INSTRUCTORS"
"STUDENTS"
]
},
"qn7": {
Expand Down Expand Up @@ -425,7 +421,6 @@
"STUDENTS"
],
"showGiverNameTo": [
"INSTRUCTORS",
"STUDENTS"
],
"showRecipientNameTo": [
Expand Down Expand Up @@ -1158,10 +1153,12 @@
"receiverSection": "None",
"feedbackResponseId": "2%[email protected]%[email protected]",
"showCommentTo": [
"STUDENTS"
"STUDENTS",
"INSTRUCTORS"
],
"showGiverNameTo": [
"STUDENTS"
"STUDENTS",
"INSTRUCTORS"
],
"commentGiverType": "INSTRUCTORS",
"isVisibilityFollowingFeedbackQuestion": false,
Expand All @@ -1178,10 +1175,12 @@
"receiverSection": "None",
"feedbackResponseId": "2%[email protected]%[email protected]",
"showCommentTo": [
"STUDENTS"
"STUDENTS",
"INSTRUCTORS"
],
"showGiverNameTo": [
"STUDENTS"
"STUDENTS",
"INSTRUCTORS"
],
"commentGiverType": "INSTRUCTORS",
"isVisibilityFollowingFeedbackQuestion": false,
Expand Down Expand Up @@ -1215,8 +1214,12 @@
"giverSection": "None",
"receiverSection": "None",
"feedbackResponseId": "3%[email protected]%[email protected]",
"showCommentTo": [],
"showGiverNameTo": [],
"showCommentTo": [
"INSTRUCTORS"
],
"showGiverNameTo": [
"INSTRUCTORS"
],
"commentGiverType": "STUDENTS",
"isVisibilityFollowingFeedbackQuestion": true,
"isCommentFromFeedbackParticipant": true,
Expand All @@ -1231,8 +1234,12 @@
"giverSection": "None",
"receiverSection": "None",
"feedbackResponseId": "4%[email protected]%%GENERAL%",
"showCommentTo": [],
"showGiverNameTo": [],
"showCommentTo": [
"INSTRUCTORS"
],
"showGiverNameTo": [
"INSTRUCTORS"
],
"commentGiverType": "STUDENTS",
"isVisibilityFollowingFeedbackQuestion": true,
"isCommentFromFeedbackParticipant": true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -17,6 +18,8 @@
public class SessionResultsBundle {

private final Map<String, FeedbackQuestionAttributes> questionsMap;
private final Map<String, FeedbackQuestionAttributes> questionsNotVisibleForPreviewMap;
private final Set<String> questionsWithCommentNotVisibleForPreview;
private final Map<String, List<FeedbackResponseAttributes>> questionResponseMap;
private final Map<String, List<FeedbackResponseAttributes>> questionMissingResponseMap;
private final Map<String, List<FeedbackResponseCommentAttributes>> responseCommentsMap;
Expand All @@ -26,6 +29,8 @@ public class SessionResultsBundle {
private final CourseRoster roster;

public SessionResultsBundle(Map<String, FeedbackQuestionAttributes> questionsMap,
Map<String, FeedbackQuestionAttributes> questionsNotVisibleForPreviewMap,
Set<String> questionsWithCommentNotVisibleForPreview,
List<FeedbackResponseAttributes> responses,
List<FeedbackResponseAttributes> missingResponses,
Map<String, Boolean> responseGiverVisibilityTable,
Expand All @@ -35,6 +40,8 @@ public SessionResultsBundle(Map<String, FeedbackQuestionAttributes> questionsMap
CourseRoster roster) {

this.questionsMap = questionsMap;
this.questionsNotVisibleForPreviewMap = questionsNotVisibleForPreviewMap;
this.questionsWithCommentNotVisibleForPreview = questionsWithCommentNotVisibleForPreview;
this.responseCommentsMap = responseCommentsMap;
this.responseGiverVisibilityTable = responseGiverVisibilityTable;
this.responseRecipientVisibilityTable = responseRecipientVisibilityTable;
Expand Down Expand Up @@ -137,6 +144,14 @@ public Map<String, FeedbackQuestionAttributes> getQuestionsMap() {
return questionsMap;
}

public Map<String, FeedbackQuestionAttributes> getQuestionsNotVisibleForPreviewMap() {
return questionsNotVisibleForPreviewMap;
}

public Set<String> getQuestionsWithCommentNotVisibleForPreview() {
return questionsWithCommentNotVisibleForPreview;
}

public Map<String, List<FeedbackResponseCommentAttributes>> getResponseCommentsMap() {
return responseCommentsMap;
}
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/teammates/logic/api/Logic.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
Loading