Skip to content

Commit

Permalink
[TEAMMATES#12048] Fix get feedback sessions action (TEAMMATES#12886)
Browse files Browse the repository at this point in the history 
* fix getFeedbackSessionsAction to support dual DB

* enable GetFeedbackSessionsAction test

* change verify course panel to search all panels rather than assume ordering

---------

Co-authored-by: Zhang Ziqing <69516975+ziqing26@users.noreply.github.com>
  • Loading branch information
@cedricongjh @ziqing26
cedricongjh and ziqing26 authored Mar 12, 2024
1 parent 3e1718c commit e385eed
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 146 deletions.
10 changes: 4 additions & 6 deletions src/e2e/java/teammates/e2e/cases/StudentHomePageE2ETest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,16 @@ public void testAll() {
______TS("courses visible to student are shown");
List<String> courseIds = getAllVisibleCourseIds();

for (int i = 0; i < courseIds.size(); i++) {
String courseId = courseIds.get(i);

homePage.verifyVisibleCourseToStudents(courseId, i);
courseIds.forEach(courseId -> {
int panelIndex = homePage.getStudentHomeCoursePanelIndex(courseId);

String feedbackSessionName = testData.feedbackSessions.entrySet().stream()
.filter(feedbackSession -> courseId.equals(feedbackSession.getValue().getCourseId()))
.map(x -> x.getValue().getFeedbackSessionName())
.collect(Collectors.joining());

homePage.verifyVisibleFeedbackSessionToStudents(feedbackSessionName, i);
}
homePage.verifyVisibleFeedbackSessionToStudents(feedbackSessionName, panelIndex);
});

______TS("notification banner is visible");
assertTrue(homePage.isBannerVisible());
Expand Down
10 changes: 4 additions & 6 deletions src/e2e/java/teammates/e2e/cases/sql/StudentHomePageE2ETest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,16 @@ public void testAll() {
______TS("courses visible to student are shown");
List<String> courseIds = getAllVisibleCourseIds();

for (int i = 0; i < courseIds.size(); i++) {
String courseId = courseIds.get(i);

homePage.verifyVisibleCourseToStudents(courseId, i);
courseIds.forEach(courseId -> {
int panelIndex = homePage.getStudentHomeCoursePanelIndex(courseId);

String feedbackSessionName = testData.feedbackSessions.entrySet().stream()
.filter(feedbackSession -> courseId.equals(feedbackSession.getValue().getCourse().getId()))
.map(x -> x.getValue().getName())
.collect(Collectors.joining());

homePage.verifyVisibleFeedbackSessionToStudents(feedbackSessionName, i);
}
homePage.verifyVisibleFeedbackSessionToStudents(feedbackSessionName, panelIndex);
});

______TS("notification banner is visible");
assertTrue(homePage.isBannerVisible());
Expand Down
12 changes: 10 additions & 2 deletions src/e2e/java/teammates/e2e/pageobjects/StudentHomePage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,16 @@ private List<WebElement> getStudentHomeCoursePanels() {
return browser.driver.findElements(By.cssSelector("div.card.bg-light"));
}

public void verifyVisibleCourseToStudents(String courseName, int index) {
assertTrue(getStudentHomeCoursePanels().get(index).getText().contains(courseName));
public int getStudentHomeCoursePanelIndex(String courseName) {
List<WebElement> coursePanels = getStudentHomeCoursePanels();
int coursePanelIndex = -1;
for (int i = 0; i < coursePanels.size(); i++) {
if (coursePanels.get(i).getText().contains(courseName)) {
coursePanelIndex = i;
}
}
assertTrue(coursePanelIndex >= 0);
return coursePanelIndex;
}

public void verifyVisibleFeedbackSessionToStudents(String feedbackSessionName, int index) {
Expand Down
211 changes: 80 additions & 131 deletions src/main/java/teammates/ui/webapi/GetFeedbackSessionsAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
import teammates.common.datatransfer.attributes.StudentAttributes;
import teammates.common.util.Const;
import teammates.storage.sqlentity.Course;
import teammates.storage.sqlentity.FeedbackSession;
import teammates.storage.sqlentity.Instructor;
import teammates.storage.sqlentity.Student;
import teammates.ui.output.FeedbackSessionData;
import teammates.ui.output.FeedbackSessionsData;

Expand Down Expand Up @@ -80,123 +83,30 @@ public JsonResult execute() {
String courseId = getRequestParamValue(Const.ParamsNames.COURSE_ID);
String entityType = getNonNullRequestParamValue(Const.ParamsNames.ENTITY_TYPE);

// TODO: revisit this for when courses are migrated, this check is not needed as all accounts are migrated
// if (isAccountMigrated(userInfo.getId())) {
// List<FeedbackSession> feedbackSessions = new ArrayList<>();
// List<Instructor> instructors = new ArrayList<>();
// List<FeedbackSessionAttributes> feedbackSessionAttributes = new ArrayList<>();
// List<String> studentEmails = new ArrayList<>();

// if (courseId == null) {
// if (entityType.equals(Const.EntityType.STUDENT)) {
// List<Student> students = sqlLogic.getStudentsByGoogleId(userInfo.getId());
// feedbackSessions = new ArrayList<>();
// for (Student student : students) {
// String studentCourseId = student.getCourse().getId();
// String emailAddress = student.getEmail();

// studentEmails.add(emailAddress);
// if (isCourseMigrated(studentCourseId)) {
// List<FeedbackSession> sessions = sqlLogic.getFeedbackSessionsForCourse(studentCourseId);

// feedbackSessions.addAll(sessions);
// } else {
// List<FeedbackSessionAttributes> sessions = logic.getFeedbackSessionsForCourse(studentCourseId);

// feedbackSessionAttributes.addAll(sessions);
// }
// }
// } else if (entityType.equals(Const.EntityType.INSTRUCTOR)) {
// boolean isInRecycleBin = getBooleanRequestParamValue(Const.ParamsNames.IS_IN_RECYCLE_BIN);

// instructors = sqlLogic.getInstructorsForGoogleId(userInfo.getId());

// if (isInRecycleBin) {
// feedbackSessions = sqlLogic.getSoftDeletedFeedbackSessionsForInstructors(instructors);
// } else {
// feedbackSessions = sqlLogic.getFeedbackSessionsForInstructors(instructors);
// }
// }
// } else {
// if (isCourseMigrated(courseId)) {
// feedbackSessions = sqlLogic.getFeedbackSessionsForCourse(courseId);
// if (entityType.equals(Const.EntityType.STUDENT) && !feedbackSessions.isEmpty()) {
// Student student = sqlLogic.getStudentByGoogleId(courseId, userInfo.getId());
// assert student != null;
// String emailAddress = student.getEmail();

// studentEmails.add(emailAddress);
// } else if (entityType.equals(Const.EntityType.INSTRUCTOR)) {
// instructors = Collections.singletonList(
// sqlLogic.getInstructorByGoogleId(courseId, userInfo.getId()));
// }
// } else {
// feedbackSessionAttributes = logic.getFeedbackSessionsForCourse(courseId);
// if (entityType.equals(Const.EntityType.STUDENT) && !feedbackSessionAttributes.isEmpty()) {
// Student student = sqlLogic.getStudentByGoogleId(courseId, userInfo.getId());
// assert student != null;
// String emailAddress = student.getEmail();
// feedbackSessionAttributes = feedbackSessionAttributes.stream()
// .map(instructorSession -> instructorSession.getCopyForStudent(emailAddress))
// .collect(Collectors.toList());
// } else if (entityType.equals(Const.EntityType.INSTRUCTOR)) {
// instructors = Collections.singletonList(
// sqlLogic.getInstructorByGoogleId(courseId, userInfo.getId()));
// }
// }
// }

// if (entityType.equals(Const.EntityType.STUDENT)) {
// // hide session not visible to student
// feedbackSessions = feedbackSessions.stream()
// .filter(FeedbackSession::isVisible).collect(Collectors.toList());
// feedbackSessionAttributes = feedbackSessionAttributes.stream()
// .filter(FeedbackSessionAttributes::isVisible).collect(Collectors.toList());
// }

// Map<String, Instructor> courseIdToInstructor = new HashMap<>();
// instructors.forEach(instructor -> courseIdToInstructor.put(instructor.getCourseId(), instructor));

// FeedbackSessionsData responseData =
// new FeedbackSessionsData(feedbackSessions, feedbackSessionAttributes);

// for (String studentEmail : studentEmails) {
// responseData.hideInformationForStudent(studentEmail);
// }

// if (entityType.equals(Const.EntityType.STUDENT)) {
// responseData.getFeedbackSessions().forEach(FeedbackSessionData::hideInformationForStudent);
// } else if (entityType.equals(Const.EntityType.INSTRUCTOR)) {
// responseData.getFeedbackSessions().forEach(session -> {
// Instructor instructor = courseIdToInstructor.get(session.getCourseId());
// if (instructor == null) {
// return;
// }

// InstructorPermissionSet privilege =
// constructInstructorPrivileges(instructor, session.getFeedbackSessionName());
// session.setPrivileges(privilege);
// });
// }
// return new JsonResult(responseData);
// } else {
return executeOldFeedbackSession(courseId, entityType);
// }
}

private JsonResult executeOldFeedbackSession(String courseId, String entityType) {
List<FeedbackSessionAttributes> feedbackSessionAttributes;
List<InstructorAttributes> instructors = new ArrayList<>();
List<FeedbackSession> feedbackSessions = new ArrayList<>();
List<InstructorAttributes> dataStoreInstructors = new ArrayList<>();
List<Instructor> instructors = new ArrayList<>();
List<FeedbackSessionAttributes> feedbackSessionAttributes = new ArrayList<>();
List<String> studentEmails = new ArrayList<>();

if (courseId == null) {
if (entityType.equals(Const.EntityType.STUDENT)) {
List<StudentAttributes> students = logic.getStudentsForGoogleId(userInfo.getId());
feedbackSessionAttributes = new ArrayList<>();
for (StudentAttributes student : students) {
List<Student> students = sqlLogic.getStudentsByGoogleId(userInfo.getId());
for (Student student : students) {
String studentCourseId = student.getCourse().getId();
String emailAddress = student.getEmail();

studentEmails.add(emailAddress);
List<FeedbackSession> sessions = sqlLogic.getFeedbackSessionsForCourse(studentCourseId);
feedbackSessions.addAll(sessions);
}
List<StudentAttributes> dataStoreStudents = logic.getStudentsForGoogleId(userInfo.getId());
for (StudentAttributes student : dataStoreStudents) {
String studentCourseId = student.getCourse();
String emailAddress = student.getEmail();
List<FeedbackSessionAttributes> sessions = logic.getFeedbackSessionsForCourse(studentCourseId);

studentEmails.add(emailAddress);
List<FeedbackSessionAttributes> sessions = logic.getFeedbackSessionsForCourse(studentCourseId);
sessions = sessions.stream()
.map(session -> session.getCopyForStudent(emailAddress))
.collect(Collectors.toList());
Expand All @@ -206,46 +116,86 @@ private JsonResult executeOldFeedbackSession(String courseId, String entityType)
} else if (entityType.equals(Const.EntityType.INSTRUCTOR)) {
boolean isInRecycleBin = getBooleanRequestParamValue(Const.ParamsNames.IS_IN_RECYCLE_BIN);

instructors = logic.getInstructorsForGoogleId(userInfo.getId(), true);
instructors = sqlLogic.getInstructorsForGoogleId(userInfo.getId());

if (isInRecycleBin) {
feedbackSessionAttributes = logic.getSoftDeletedFeedbackSessionsListForInstructors(instructors);
feedbackSessions = sqlLogic.getSoftDeletedFeedbackSessionsForInstructors(instructors);
} else {
feedbackSessionAttributes = logic.getFeedbackSessionsListForInstructor(instructors);
feedbackSessions = sqlLogic.getFeedbackSessionsForInstructors(instructors);
}

dataStoreInstructors = logic.getInstructorsForGoogleId(userInfo.getId(), true);

if (isInRecycleBin) {
feedbackSessionAttributes = logic.getSoftDeletedFeedbackSessionsListForInstructors(dataStoreInstructors);
} else {
feedbackSessionAttributes = logic.getFeedbackSessionsListForInstructor(dataStoreInstructors);
}
} else {
feedbackSessionAttributes = new ArrayList<>();
}
} else {
feedbackSessionAttributes = logic.getFeedbackSessionsForCourse(courseId);
if (entityType.equals(Const.EntityType.STUDENT) && !feedbackSessionAttributes.isEmpty()) {
StudentAttributes student = logic.getStudentForGoogleId(courseId, userInfo.getId());
assert student != null;
String emailAddress = student.getEmail();
feedbackSessionAttributes = feedbackSessionAttributes.stream()
.map(instructorSession -> instructorSession.getCopyForStudent(emailAddress))
.collect(Collectors.toList());
} else if (entityType.equals(Const.EntityType.INSTRUCTOR)) {
instructors = Collections.singletonList(logic.getInstructorForGoogleId(courseId, userInfo.getId()));
if (isCourseMigrated(courseId)) {
feedbackSessions = sqlLogic.getFeedbackSessionsForCourse(courseId);
if (entityType.equals(Const.EntityType.STUDENT) && !feedbackSessions.isEmpty()) {
Student student = sqlLogic.getStudentByGoogleId(courseId, userInfo.getId());
assert student != null;
String emailAddress = student.getEmail();

studentEmails.add(emailAddress);
} else if (entityType.equals(Const.EntityType.INSTRUCTOR)) {
instructors = Collections.singletonList(
sqlLogic.getInstructorByGoogleId(courseId, userInfo.getId()));
}
} else {
feedbackSessionAttributes = logic.getFeedbackSessionsForCourse(courseId);
if (entityType.equals(Const.EntityType.STUDENT) && !feedbackSessionAttributes.isEmpty()) {
StudentAttributes student = logic.getStudentForGoogleId(courseId, userInfo.getId());
assert student != null;
String emailAddress = student.getEmail();
feedbackSessionAttributes = feedbackSessionAttributes.stream()
.map(instructorSession -> instructorSession.getCopyForStudent(emailAddress))
.collect(Collectors.toList());
} else if (entityType.equals(Const.EntityType.INSTRUCTOR)) {
dataStoreInstructors =
Collections.singletonList(logic.getInstructorForGoogleId(courseId, userInfo.getId()));
}
}
}

if (entityType.equals(Const.EntityType.STUDENT)) {
// hide session not visible to student
feedbackSessions = feedbackSessions.stream()
.filter(FeedbackSession::isVisible).collect(Collectors.toList());
feedbackSessionAttributes = feedbackSessionAttributes.stream()
.filter(FeedbackSessionAttributes::isVisible).collect(Collectors.toList());
}

Map<String, InstructorAttributes> courseIdToInstructor = new HashMap<>();
Map<String, Instructor> courseIdToInstructor = new HashMap<>();
instructors.forEach(instructor -> courseIdToInstructor.put(instructor.getCourseId(), instructor));

FeedbackSessionsData responseData = new FeedbackSessionsData(feedbackSessionAttributes);
Map<String, InstructorAttributes> dataStoreCourseIdToInstructor = new HashMap<>();
dataStoreInstructors.forEach(instructor -> dataStoreCourseIdToInstructor.put(instructor.getCourseId(), instructor));

FeedbackSessionsData responseData =
new FeedbackSessionsData(feedbackSessions, feedbackSessionAttributes);

for (String studentEmail : studentEmails) {
responseData.hideInformationForStudent(studentEmail);
}

if (entityType.equals(Const.EntityType.STUDENT)) {
responseData.getFeedbackSessions().forEach(FeedbackSessionData::hideInformationForStudent);
} else if (entityType.equals(Const.EntityType.INSTRUCTOR)) {
responseData.getFeedbackSessions().forEach(session -> {
InstructorAttributes instructor = courseIdToInstructor.get(session.getCourseId());
if (instructor == null) {
Instructor instructor = courseIdToInstructor.get(session.getCourseId());
InstructorAttributes dataStoreInstructor = dataStoreCourseIdToInstructor.get(session.getCourseId());
if (instructor == null && dataStoreInstructor == null) {
return;
}

if (dataStoreInstructor != null) {
InstructorPermissionSet privilege =
constructInstructorPrivileges(dataStoreInstructor, session.getFeedbackSessionName());
session.setPrivileges(privilege);
return;
}

Expand All @@ -256,5 +206,4 @@ private JsonResult executeOldFeedbackSession(String courseId, String entityType)
}
return new JsonResult(responseData);
}

}
2 changes: 1 addition & 1 deletion src/test/java/teammates/sqlui/webapi/GetFeedbackSessionsActionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ void setUp() {
instructor1.getAccount().getGoogleId(), course1.getId())).thenReturn(instructor1);
}

@Test(enabled = false) // enable once we are migrating course data to sql
@Test
protected void textExecute() {
loginAsStudent(student1.getAccount().getGoogleId());

Expand Down

0 comments on commit e385eed

Please sign in to comment.