diff --git a/src/it/java/teammates/it/ui/webapi/GetInstructorsActionIT.java b/src/it/java/teammates/it/ui/webapi/GetInstructorsActionIT.java new file mode 100644 index 00000000000..2468db40ca1 --- /dev/null +++ b/src/it/java/teammates/it/ui/webapi/GetInstructorsActionIT.java @@ -0,0 +1,173 @@ +package teammates.it.ui.webapi; + +import java.util.List; + +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import teammates.common.util.Const; +import teammates.common.util.HibernateUtil; +import teammates.storage.sqlentity.Instructor; +import teammates.storage.sqlentity.Student; +import teammates.ui.output.InstructorData; +import teammates.ui.output.InstructorsData; +import teammates.ui.request.Intent; +import teammates.ui.webapi.GetInstructorsAction; +import teammates.ui.webapi.JsonResult; + +/** + * SUT: {@link GetInstructorsAction}. + */ +public class GetInstructorsActionIT extends BaseActionIT { + + @Override + @BeforeMethod + protected void setUp() throws Exception { + super.setUp(); + persistDataBundle(typicalBundle); + HibernateUtil.flushSession(); + } + + @Override + protected String getActionUri() { + return Const.ResourceURIs.INSTRUCTORS; + } + + @Override + protected String getRequestMethod() { + return GET; + } + + @Test + @Override + protected void testExecute() throws Exception { + Instructor instructor = typicalBundle.instructors.get("instructor1OfCourse1"); + + loginAsInstructor(instructor.getGoogleId()); + + ______TS("Typical Success Case with FULL_DETAIL"); + String[] params = new String[] { + Const.ParamsNames.COURSE_ID, instructor.getCourseId(), + Const.ParamsNames.INTENT, Intent.FULL_DETAIL.toString(), + }; + + GetInstructorsAction action = getAction(params); + JsonResult jsonResult = getJsonResult(action); + + InstructorsData output = (InstructorsData) jsonResult.getOutput(); + List instructors = output.getInstructors(); + + assertEquals(2, instructors.size()); + + ______TS("Typical Success Case with no intent"); + params = new String[] { + Const.ParamsNames.COURSE_ID, instructor.getCourseId(), + Const.ParamsNames.INTENT, null, + }; + + action = getAction(params); + jsonResult = getJsonResult(action); + + output = (InstructorsData) jsonResult.getOutput(); + instructors = output.getInstructors(); + + assertEquals(2, instructors.size()); + + for (InstructorData instructorData : instructors) { + assertNull(instructorData.getGoogleId()); + assertNull(instructorData.getJoinState()); + assertNull(instructorData.getIsDisplayedToStudents()); + assertNull(instructorData.getRole()); + } + + ______TS("Unknown intent"); + params = new String[] { + Const.ParamsNames.COURSE_ID, instructor.getCourseId(), + Const.ParamsNames.INTENT, "Unknown", + }; + + verifyHttpParameterFailure(params); + } + + @Test + @Override + protected void testAccessControl() throws Exception { + Instructor instructor = typicalBundle.instructors.get("instructor1OfCourse1"); + Student student = typicalBundle.students.get("student1InCourse1"); + + ______TS("Course not found, logged in as instructor, intent FULL_DETAIL"); + loginAsInstructor(instructor.getGoogleId()); + + String[] params = new String[] { + Const.ParamsNames.COURSE_ID, "does-not-exist-id", + Const.ParamsNames.INTENT, Intent.FULL_DETAIL.toString(), + }; + + verifyEntityNotFoundAcl(params); + + ______TS("Course not found, logged in as student, intent undefined"); + loginAsStudent(student.getGoogleId()); + + params = new String[] { + Const.ParamsNames.COURSE_ID, "does-not-exist-id", + }; + + verifyEntityNotFoundAcl(params); + + ______TS("Unknown login entity, intent FULL_DETAIL"); + loginAsUnregistered("unregistered"); + + params = new String[] { + Const.ParamsNames.COURSE_ID, instructor.getCourseId(), + Const.ParamsNames.INTENT, Intent.FULL_DETAIL.toString(), + }; + + verifyCannotAccess(params); + + ______TS("Unknown login entity, intent undefined"); + params = new String[] { + Const.ParamsNames.COURSE_ID, instructor.getCourseId(), + }; + + verifyCannotAccess(params); + + ______TS("Unknown intent, logged in as instructor"); + loginAsInstructor(instructor.getGoogleId()); + + params = new String[] { + Const.ParamsNames.COURSE_ID, instructor.getCourseId(), + Const.ParamsNames.INTENT, "Unknown", + }; + + verifyHttpParameterFailureAcl(params); + + ______TS("Intent FULL_DETAIL, should authenticate as instructor"); + params = new String[] { + Const.ParamsNames.COURSE_ID, instructor.getCourseId(), + Const.ParamsNames.INTENT, Intent.FULL_DETAIL.toString(), + }; + + verifyOnlyInstructorsOfTheSameCourseCanAccess(instructor.getCourse(), params); + + ______TS("Intent undefined, should authenticate as student, access own course"); + loginAsStudent(student.getGoogleId()); + + params = new String[] { + Const.ParamsNames.COURSE_ID, student.getCourseId(), + }; + + verifyCanAccess(params); + + ______TS("Intent undefined, should authenticate as student, access other course"); + Student otherStudent = typicalBundle.students.get("student1InCourse2"); + + assertNotEquals(otherStudent.getCourse(), student.getCourse()); + + params = new String[] { + Const.ParamsNames.COURSE_ID, otherStudent.getCourseId(), + }; + + verifyCannotAccess(params); + } + +} diff --git a/src/it/resources/data/typicalDataBundle.json b/src/it/resources/data/typicalDataBundle.json index 55de3855801..63fcbeb7e5e 100644 --- a/src/it/resources/data/typicalDataBundle.json +++ b/src/it/resources/data/typicalDataBundle.json @@ -57,6 +57,13 @@ "id": "course-1" }, "name": "Section 1" + }, + "section1InCourse2": { + "id": "00000000-0000-4000-8000-000000000202", + "course": { + "id": "idOfCourse2" + }, + "name": "Section 2" } }, "teams": { @@ -66,6 +73,13 @@ "id": "00000000-0000-4000-8000-000000000201" }, "name": "Team 1" + }, + "team1InCourse2": { + "id": "00000000-0000-4000-8000-000000000302", + "section": { + "id": "00000000-0000-4000-8000-000000000202" + }, + "name": "Team 1" } }, "deadlineExtensions": { @@ -174,6 +188,18 @@ "email": "student2@teammates.tmt", "name": "student2 In Course1", "comments": "" + }, + "student1InCourse2": { + "id": "00000000-0000-4000-8000-000000000603", + "course": { + "id": "idOfCourse2" + }, + "team": { + "id": "00000000-0000-4000-8000-000000000302" + }, + "email": "student1@teammates.tmt", + "name": "student1 In Course2", + "comments": "" } }, "feedbackSessions": { diff --git a/src/main/java/teammates/sqllogic/api/Logic.java b/src/main/java/teammates/sqllogic/api/Logic.java index 9f2fbf04f39..6c9b6318452 100644 --- a/src/main/java/teammates/sqllogic/api/Logic.java +++ b/src/main/java/teammates/sqllogic/api/Logic.java @@ -389,6 +389,13 @@ public Instructor getInstructorByGoogleId(String courseId, String googleId) { return usersLogic.getInstructorByGoogleId(courseId, googleId); } + /** + * Gets instructors by associated {@code courseId}. + */ + public List getInstructorsByCourse(String courseId) { + return usersLogic.getInstructorsForCourse(courseId); + } + /** * Creates an instructor. */ diff --git a/src/main/java/teammates/ui/output/InstructorsData.java b/src/main/java/teammates/ui/output/InstructorsData.java index c886c346698..a546b36d488 100644 --- a/src/main/java/teammates/ui/output/InstructorsData.java +++ b/src/main/java/teammates/ui/output/InstructorsData.java @@ -4,7 +4,7 @@ import java.util.List; import java.util.stream.Collectors; -import teammates.common.datatransfer.attributes.InstructorAttributes; +import teammates.storage.sqlentity.Instructor; /** * The API output format of a list of instructors. @@ -17,8 +17,8 @@ public InstructorsData() { this.instructors = new ArrayList<>(); } - public InstructorsData(List instructorAttributesList) { - this.instructors = instructorAttributesList.stream().map(InstructorData::new).collect(Collectors.toList()); + public InstructorsData(List instructorsList) { + this.instructors = instructorsList.stream().map(InstructorData::new).collect(Collectors.toList()); } public List getInstructors() { diff --git a/src/main/java/teammates/ui/webapi/GetInstructorsAction.java b/src/main/java/teammates/ui/webapi/GetInstructorsAction.java index 021b1231c7e..8094ac8b1ee 100644 --- a/src/main/java/teammates/ui/webapi/GetInstructorsAction.java +++ b/src/main/java/teammates/ui/webapi/GetInstructorsAction.java @@ -7,6 +7,9 @@ import teammates.common.datatransfer.attributes.InstructorAttributes; import teammates.common.datatransfer.attributes.StudentAttributes; import teammates.common.util.Const; +import teammates.storage.sqlentity.Course; +import teammates.storage.sqlentity.Instructor; +import teammates.storage.sqlentity.Student; import teammates.ui.output.InstructorData; import teammates.ui.output.InstructorsData; import teammates.ui.request.Intent; @@ -14,7 +17,7 @@ /** * Get a list of instructors of a course. */ -class GetInstructorsAction extends Action { +public class GetInstructorsAction extends Action { @Override AuthType getMinAuthLevel() { @@ -28,41 +31,120 @@ void checkSpecificAccessControl() throws UnauthorizedAccessException { } String courseId = getNonNullRequestParamValue(Const.ParamsNames.COURSE_ID); - CourseAttributes course = logic.getCourse(courseId); - if (course == null) { - throw new EntityNotFoundException("course not found"); - } - String intentStr = getRequestParamValue(Const.ParamsNames.INTENT); - if (intentStr == null) { - // get partial details of instructors with information hiding - // student should belong to the course - StudentAttributes student = logic.getStudentForGoogleId(courseId, userInfo.getId()); - gateKeeper.verifyAccessible(student, course); - } else if (intentStr.equals(Intent.FULL_DETAIL.toString())) { - // get all instructors of a course without information hiding - // this need instructor privileges - InstructorAttributes instructor = logic.getInstructorForGoogleId(courseId, userInfo.getId()); - gateKeeper.verifyAccessible(instructor, course); + if (isCourseMigrated(courseId)) { + Course course = sqlLogic.getCourse(courseId); + + if (course == null) { + throw new EntityNotFoundException("course not found"); + } + + String intentStr = getRequestParamValue(Const.ParamsNames.INTENT); + + if (intentStr == null) { + // get partial details of instructors with information hiding + // student should belong to the course + Student student = sqlLogic.getStudentByGoogleId(courseId, userInfo.getId()); + gateKeeper.verifyAccessible(student, course); + } else if (intentStr.equals(Intent.FULL_DETAIL.toString())) { + // get all instructors of a course without information hiding + // this need instructor privileges + Instructor instructor = sqlLogic.getInstructorByGoogleId(courseId, userInfo.getId()); + gateKeeper.verifyAccessible(instructor, course); + } else { + throw new InvalidHttpParameterException("unknown intent"); + } } else { - throw new InvalidHttpParameterException("unknown intent"); - } + CourseAttributes course = logic.getCourse(courseId); + if (course == null) { + throw new EntityNotFoundException("course not found"); + } + String intentStr = getRequestParamValue(Const.ParamsNames.INTENT); + if (intentStr == null) { + // get partial details of instructors with information hiding + // student should belong to the course + StudentAttributes student = logic.getStudentForGoogleId(courseId, userInfo.getId()); + gateKeeper.verifyAccessible(student, course); + } else if (intentStr.equals(Intent.FULL_DETAIL.toString())) { + // get all instructors of a course without information hiding + // this need instructor privileges + InstructorAttributes instructor = logic.getInstructorForGoogleId(courseId, userInfo.getId()); + gateKeeper.verifyAccessible(instructor, course); + } else { + throw new InvalidHttpParameterException("unknown intent"); + } + } } @Override public JsonResult execute() { String courseId = getNonNullRequestParamValue(Const.ParamsNames.COURSE_ID); - List instructorsOfCourse = logic.getInstructorsForCourse(courseId); - + String intentStr = getRequestParamValue(Const.ParamsNames.INTENT); InstructorsData data; - String intentStr = getRequestParamValue(Const.ParamsNames.INTENT); - if (intentStr == null) { - instructorsOfCourse = - instructorsOfCourse.stream() - .filter(InstructorAttributes::isDisplayedToStudents) + if (!isCourseMigrated(courseId)) { + List instructorsOfCourse = logic.getInstructorsForCourse(courseId); + + if (intentStr == null) { + data = new InstructorsData(); + instructorsOfCourse = + instructorsOfCourse.stream() + .filter(InstructorAttributes::isDisplayedToStudents) + .collect(Collectors.toList()); + + List instructorDataList = instructorsOfCourse + .stream() + .map(InstructorData::new) + .collect(Collectors.toList()); + + data.setInstructors(instructorDataList); + + // hide information + data.getInstructors().forEach(i -> { + i.setGoogleId(null); + i.setJoinState(null); + i.setIsDisplayedToStudents(null); + i.setRole(null); + }); + } else if (intentStr.equals(Intent.FULL_DETAIL.toString())) { + // get all instructors of a course without information hiding + // adds googleId if caller is admin or has the appropriate privilege to modify instructor + if (userInfo.isAdmin || logic.getInstructorForGoogleId(courseId, userInfo.getId()).getPrivileges() + .isAllowedForPrivilege(Const.InstructorPermissions.CAN_MODIFY_INSTRUCTOR)) { + data = new InstructorsData(); + for (InstructorAttributes instructor : instructorsOfCourse) { + InstructorData instructorData = new InstructorData(instructor); + instructorData.setGoogleId(instructor.getGoogleId()); + if (userInfo.isAdmin) { + instructorData.setKey(instructor.getKey()); + } + data.getInstructors().add(instructorData); + } + } else { + data = new InstructorsData(); + + List instructorDataList = instructorsOfCourse + .stream() + .map(InstructorData::new) .collect(Collectors.toList()); + + data.setInstructors(instructorDataList); + } + } else { + throw new InvalidHttpParameterException("unknown intent"); + } + + return new JsonResult(data); + } + + List instructorsOfCourse = sqlLogic.getInstructorsByCourse(courseId); + + if (intentStr == null) { + instructorsOfCourse = instructorsOfCourse + .stream() + .filter(Instructor::isDisplayedToStudents) + .collect(Collectors.toList()); data = new InstructorsData(instructorsOfCourse); // hide information @@ -75,14 +157,15 @@ public JsonResult execute() { } else if (intentStr.equals(Intent.FULL_DETAIL.toString())) { // get all instructors of a course without information hiding // adds googleId if caller is admin or has the appropriate privilege to modify instructor - if (userInfo.isAdmin || logic.getInstructorForGoogleId(courseId, userInfo.getId()).getPrivileges() + if (userInfo.isAdmin || sqlLogic.getInstructorByGoogleId(courseId, userInfo.getId()).getPrivileges() .isAllowedForPrivilege(Const.InstructorPermissions.CAN_MODIFY_INSTRUCTOR)) { data = new InstructorsData(); - for (InstructorAttributes instructor : instructorsOfCourse) { + + for (Instructor instructor : instructorsOfCourse) { InstructorData instructorData = new InstructorData(instructor); instructorData.setGoogleId(instructor.getGoogleId()); if (userInfo.isAdmin) { - instructorData.setKey(instructor.getKey()); + instructorData.setKey(instructor.getRegKey()); } data.getInstructors().add(instructorData); } diff --git a/src/test/java/teammates/ui/webapi/GetInstructorsActionTest.java b/src/test/java/teammates/ui/webapi/GetInstructorsActionTest.java index f692e1e8965..fd0cb6089f8 100644 --- a/src/test/java/teammates/ui/webapi/GetInstructorsActionTest.java +++ b/src/test/java/teammates/ui/webapi/GetInstructorsActionTest.java @@ -2,6 +2,7 @@ import java.util.List; +import org.testng.annotations.Ignore; import org.testng.annotations.Test; import teammates.common.datatransfer.InstructorPermissionRole; @@ -16,6 +17,7 @@ /** * SUT: {@link GetInstructorsAction}. */ +@Ignore public class GetInstructorsActionTest extends BaseActionTest { @Override