diff --git a/src/it/java/teammates/it/storage/sqlapi/AccountRequestsDbIT.java b/src/it/java/teammates/it/storage/sqlapi/AccountRequestsDbIT.java index baddc6b3a9f..7214b6a08ac 100644 --- a/src/it/java/teammates/it/storage/sqlapi/AccountRequestsDbIT.java +++ b/src/it/java/teammates/it/storage/sqlapi/AccountRequestsDbIT.java @@ -33,7 +33,7 @@ public void testCreateReadDeleteAccountRequest() throws Exception { ______TS("Read account request using the given registration key"); AccountRequest actualAccReqRegistrationKey = - accountRequestDb.getAccountRequest(accountRequest.getRegistrationKey()); + accountRequestDb.getAccountRequestByRegistrationKey(accountRequest.getRegistrationKey()); verifyEquals(accountRequest, actualAccReqRegistrationKey); ______TS("Read account request using the given start and end timing"); diff --git a/src/it/java/teammates/it/storage/sqlsearch/InstructorSearchIT.java b/src/it/java/teammates/it/storage/sqlsearch/InstructorSearchIT.java index 0c3c6c0ad38..b8591714ecb 100644 --- a/src/it/java/teammates/it/storage/sqlsearch/InstructorSearchIT.java +++ b/src/it/java/teammates/it/storage/sqlsearch/InstructorSearchIT.java @@ -45,6 +45,7 @@ public void allTests() throws Exception { Instructor insInUnregCourse = typicalBundle.instructors.get("instructorOfUnregisteredCourse"); Instructor insUniqueDisplayName = typicalBundle.instructors.get("instructorOfCourse2WithUniqueDisplayName"); Instructor ins1InCourse3 = typicalBundle.instructors.get("instructor1OfCourse3"); + Instructor unregisteredInsInCourse1 = typicalBundle.instructors.get("unregisteredInstructorOfCourse1"); ______TS("success: search for instructors in whole system; query string does not match anyone"); @@ -80,12 +81,12 @@ public void allTests() throws Exception { ______TS("success: search for instructors in whole system; instructors should be searchable by course id"); results = usersDb.searchInstructorsInWholeSystem("\"course-1\""); - verifySearchResults(results, ins1InCourse1, ins2InCourse1); + verifySearchResults(results, ins1InCourse1, ins2InCourse1, unregisteredInsInCourse1); ______TS("success: search for instructors in whole system; instructors should be searchable by course name"); results = usersDb.searchInstructorsInWholeSystem("\"Typical Course 1\""); - verifySearchResults(results, ins1InCourse1, ins2InCourse1); + verifySearchResults(results, ins1InCourse1, ins2InCourse1, unregisteredInsInCourse1); ______TS("success: search for instructors in whole system; instructors should be searchable by their name"); @@ -136,17 +137,22 @@ public void testSearchInstructor_deleteAfterSearch_shouldNotBeSearchable() throw Instructor ins1InCourse1 = typicalBundle.instructors.get("instructor1OfCourse1"); Instructor ins2InCourse1 = typicalBundle.instructors.get("instructor2OfCourse1"); + Instructor unregisteredInsInCourse1 = typicalBundle.instructors.get("unregisteredInstructorOfCourse1"); List results = usersDb.searchInstructorsInWholeSystem("\"course-1\""); - verifySearchResults(results, ins1InCourse1, ins2InCourse1); + verifySearchResults(results, ins1InCourse1, ins2InCourse1, unregisteredInsInCourse1); usersDb.deleteUser(ins1InCourse1); results = usersDb.searchInstructorsInWholeSystem("\"course-1\""); - verifySearchResults(results, ins2InCourse1); + verifySearchResults(results, ins2InCourse1, unregisteredInsInCourse1); // This used to test .deleteInstructors, but we don't seem to have a similar method to delete all users in course usersDb.deleteUser(ins2InCourse1); results = usersDb.searchInstructorsInWholeSystem("\"course-1\""); + verifySearchResults(results, unregisteredInsInCourse1); + + usersDb.deleteUser(unregisteredInsInCourse1); + results = usersDb.searchInstructorsInWholeSystem("\"course-1\""); verifySearchResults(results); } diff --git a/src/it/java/teammates/it/ui/webapi/DeleteStudentsActionIT.java b/src/it/java/teammates/it/ui/webapi/DeleteStudentsActionIT.java index accb289fda1..83d4252c298 100644 --- a/src/it/java/teammates/it/ui/webapi/DeleteStudentsActionIT.java +++ b/src/it/java/teammates/it/ui/webapi/DeleteStudentsActionIT.java @@ -41,14 +41,14 @@ protected void testExecute() throws Exception { Instructor instructor = typicalBundle.instructors.get("instructor1OfCourse1"); String courseId = instructor.getCourseId(); // TODO Remove limit after migration completes - int deleteLimit = 3; + int deleteLimit = 4; ______TS("Typical Success Case delete a limited number of students"); loginAsInstructor(instructor.getGoogleId()); List studentsToDelete = logic.getStudentsForCourse(courseId); - assertEquals(3, studentsToDelete.size()); + assertEquals(4, studentsToDelete.size()); String[] params = new String[] { Const.ParamsNames.COURSE_ID, courseId, @@ -59,7 +59,7 @@ protected void testExecute() throws Exception { getJsonResult(deleteStudentsAction); for (Student student : studentsToDelete) { - assertNull(logic.getStudentByGoogleId(courseId, student.getGoogleId())); + assertNull(logic.getStudentByRegistrationKey(student.getRegKey())); } ______TS("Random course given, fails silently"); diff --git a/src/it/java/teammates/it/ui/webapi/GetCourseJoinStatusActionIT.java b/src/it/java/teammates/it/ui/webapi/GetCourseJoinStatusActionIT.java new file mode 100644 index 00000000000..893883934a8 --- /dev/null +++ b/src/it/java/teammates/it/ui/webapi/GetCourseJoinStatusActionIT.java @@ -0,0 +1,191 @@ +package teammates.it.ui.webapi; + +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import teammates.common.util.Const; +import teammates.common.util.HibernateUtil; +import teammates.ui.output.JoinStatus; +import teammates.ui.webapi.GetCourseJoinStatusAction; +import teammates.ui.webapi.JsonResult; + +/** + * SUT: {@link GetCourseJoinStatusAction}. + */ +public class GetCourseJoinStatusActionIT extends BaseActionIT { + + @Override + @BeforeMethod + protected void setUp() throws Exception { + super.setUp(); + this.typicalBundle = loadSqlDataBundle("/typicalDataBundle.json"); + persistDataBundle(typicalBundle); + HibernateUtil.flushSession(); + } + + @Override + protected String getActionUri() { + return Const.ResourceURIs.JOIN; + } + + @Override + protected String getRequestMethod() { + return GET; + } + + @Override + @Test + protected void testExecute() { + + loginAsUnregistered("unreg.user"); + + ______TS("Not enough parameters"); + + verifyHttpParameterFailure(); + verifyHttpParameterFailure( + Const.ParamsNames.REGKEY, "regkey" + ); + verifyHttpParameterFailure( + Const.ParamsNames.ENTITY_TYPE, Const.EntityType.STUDENT + ); + + ______TS("Normal case: student is already registered"); + String registeredStudentKey = + logic.getStudentForEmail("course-1", "student1@teammates.tmt").getRegKey(); + + String[] params = new String[] { + Const.ParamsNames.REGKEY, registeredStudentKey, + Const.ParamsNames.ENTITY_TYPE, Const.EntityType.STUDENT, + }; + + GetCourseJoinStatusAction getCourseJoinStatusAction = getAction(params); + JsonResult result = getJsonResult(getCourseJoinStatusAction); + + JoinStatus output = (JoinStatus) result.getOutput(); + assertTrue(output.getHasJoined()); + + ______TS("Normal case: student is not registered"); + String unregisteredStudentKey = + logic.getStudentForEmail("course-1", "unregisteredStudentInCourse1@teammates.tmt").getRegKey(); + + params = new String[] { + Const.ParamsNames.REGKEY, unregisteredStudentKey, + Const.ParamsNames.ENTITY_TYPE, Const.EntityType.STUDENT, + }; + + getCourseJoinStatusAction = getAction(params); + result = getJsonResult(getCourseJoinStatusAction); + + output = (JoinStatus) result.getOutput(); + assertFalse(output.getHasJoined()); + + ______TS("Failure case: regkey is not valid for student"); + + params = new String[] { + Const.ParamsNames.REGKEY, "ANXKJZNZXNJCZXKJDNKSDA", + Const.ParamsNames.ENTITY_TYPE, Const.EntityType.STUDENT, + }; + + verifyEntityNotFound(params); + + ______TS("Normal case: instructor is already registered"); + + String registeredInstructorKey = + logic.getInstructorForEmail("course-1", "instr1@teammates.tmt").getRegKey(); + + params = new String[] { + Const.ParamsNames.REGKEY, registeredInstructorKey, + Const.ParamsNames.ENTITY_TYPE, Const.EntityType.INSTRUCTOR, + }; + + getCourseJoinStatusAction = getAction(params); + result = getJsonResult(getCourseJoinStatusAction); + + output = (JoinStatus) result.getOutput(); + assertTrue(output.getHasJoined()); + + ______TS("Normal case: instructor is not registered"); + + String unregisteredInstructorKey = + logic.getInstructorForEmail("course-1", "unregisteredInstructor@teammates.tmt").getRegKey(); + + params = new String[] { + Const.ParamsNames.REGKEY, unregisteredInstructorKey, + Const.ParamsNames.ENTITY_TYPE, Const.EntityType.INSTRUCTOR, + }; + + getCourseJoinStatusAction = getAction(params); + result = getJsonResult(getCourseJoinStatusAction); + + output = (JoinStatus) result.getOutput(); + assertFalse(output.getHasJoined()); + + ______TS("Failure case: regkey is not valid for instructor"); + + params = new String[] { + Const.ParamsNames.REGKEY, "ANXKJZNZXNJCZXKJDNKSDA", + Const.ParamsNames.ENTITY_TYPE, Const.EntityType.INSTRUCTOR, + }; + + verifyEntityNotFound(params); + + ______TS("Normal case: account request not used, instructor has not joined course"); + + String accountRequestNotUsedKey = logic.getAccountRequest("unregisteredInstructor@teammates.tmt", + "TEAMMATES Test Institute 1").getRegistrationKey(); + + params = new String[] { + Const.ParamsNames.REGKEY, accountRequestNotUsedKey, + Const.ParamsNames.ENTITY_TYPE, Const.EntityType.INSTRUCTOR, + Const.ParamsNames.IS_CREATING_ACCOUNT, "true", + }; + + getCourseJoinStatusAction = getAction(params); + result = getJsonResult(getCourseJoinStatusAction); + + output = (JoinStatus) result.getOutput(); + assertFalse(output.getHasJoined()); + + ______TS("Normal case: account request already used, instructor has joined course"); + + String accountRequestUsedKey = + logic.getAccountRequest("instr1@teammates.tmt", "TEAMMATES Test Institute 1").getRegistrationKey(); + + params = new String[] { + Const.ParamsNames.REGKEY, accountRequestUsedKey, + Const.ParamsNames.ENTITY_TYPE, Const.EntityType.INSTRUCTOR, + Const.ParamsNames.IS_CREATING_ACCOUNT, "true", + }; + + getCourseJoinStatusAction = getAction(params); + result = getJsonResult(getCourseJoinStatusAction); + + output = (JoinStatus) result.getOutput(); + assertTrue(output.getHasJoined()); + + ______TS("Failure case: account request regkey is not valid"); + + params = new String[] { + Const.ParamsNames.REGKEY, "invalid-registration-key", + Const.ParamsNames.ENTITY_TYPE, Const.EntityType.INSTRUCTOR, + Const.ParamsNames.IS_CREATING_ACCOUNT, "true", + }; + + verifyEntityNotFound(params); + + ______TS("Failure case: invalid entity type"); + + params = new String[] { + Const.ParamsNames.REGKEY, unregisteredStudentKey, + Const.ParamsNames.ENTITY_TYPE, "unknown", + }; + + verifyHttpParameterFailure(params); + } + + @Test + @Override + protected void testAccessControl() throws Exception { + verifyAnyLoggedInUserCanAccess(); + } +} diff --git a/src/it/java/teammates/it/ui/webapi/GetInstructorsActionIT.java b/src/it/java/teammates/it/ui/webapi/GetInstructorsActionIT.java index 2468db40ca1..35ee8798edb 100644 --- a/src/it/java/teammates/it/ui/webapi/GetInstructorsActionIT.java +++ b/src/it/java/teammates/it/ui/webapi/GetInstructorsActionIT.java @@ -57,7 +57,7 @@ protected void testExecute() throws Exception { InstructorsData output = (InstructorsData) jsonResult.getOutput(); List instructors = output.getInstructors(); - assertEquals(2, instructors.size()); + assertEquals(3, instructors.size()); ______TS("Typical Success Case with no intent"); params = new String[] { @@ -71,7 +71,7 @@ protected void testExecute() throws Exception { output = (InstructorsData) jsonResult.getOutput(); instructors = output.getInstructors(); - assertEquals(2, instructors.size()); + assertEquals(3, instructors.size()); for (InstructorData instructorData : instructors) { assertNull(instructorData.getGoogleId()); diff --git a/src/it/java/teammates/it/ui/webapi/GetStudentsActionIT.java b/src/it/java/teammates/it/ui/webapi/GetStudentsActionIT.java index 161607374be..0d6464c2668 100644 --- a/src/it/java/teammates/it/ui/webapi/GetStudentsActionIT.java +++ b/src/it/java/teammates/it/ui/webapi/GetStudentsActionIT.java @@ -57,7 +57,7 @@ protected void testExecute() throws Exception { StudentsData response = (StudentsData) jsonResult.getOutput(); List students = response.getStudents(); - assertEquals(3, students.size()); + assertEquals(4, students.size()); StudentData firstStudentInStudents = students.get(0); @@ -82,7 +82,7 @@ protected void testExecute() throws Exception { Student expectedOtherTeamMember = typicalBundle.students.get("student2InCourse1"); - assertEquals(3, students.size()); + assertEquals(4, students.size()); StudentData actualOtherTeamMember = students.get(1); diff --git a/src/it/resources/data/typicalDataBundle.json b/src/it/resources/data/typicalDataBundle.json index b874b8d3ace..6b06d92b4a1 100644 --- a/src/it/resources/data/typicalDataBundle.json +++ b/src/it/resources/data/typicalDataBundle.json @@ -63,6 +63,12 @@ "email": "instr2@teammates.tmt", "institute": "TEAMMATES Test Institute 1", "registeredAt": "2015-02-14T00:00:00Z" + }, + "unregisteredInstructor": { + "id": "00000000-0000-4000-8000-000000000103", + "name": "Unregistered Instructor", + "email": "unregisteredInstructor@teammates.tmt", + "institute": "TEAMMATES Test Institute 1" } }, "courses": { @@ -328,6 +334,31 @@ "sectionLevel": {}, "sessionLevel": {} } + }, + "unregisteredInstructorOfCourse1": { + "id": "00000000-0000-4000-8000-000000000507", + "course": { + "id": "course-1" + }, + "name": "Unregistered Instructor", + "email": "unregisteredInstructor@teammates.tmt", + "role": "INSTRUCTOR_PERMISSION_ROLE_TUTOR", + "isDisplayedToStudents": true, + "displayName": "Unregistered Instructor", + "privileges": { + "courseLevel": { + "canModifyCourse": false, + "canModifyInstructor": false, + "canModifySession": false, + "canModifyStudent": false, + "canViewStudentInSections": true, + "canViewSessionInSections": true, + "canSubmitSessionInSections": true, + "canModifySessionCommentsInSections": false + }, + "sectionLevel": {}, + "sessionLevel": {} + } } }, "students": { @@ -387,6 +418,18 @@ "email": "student1@teammates.tmt", "name": "student1 In Course2", "comments": "" + }, + "unregisteredStudentInCourse1": { + "id": "00000000-0000-4000-8000-000000000605", + "course": { + "id": "course-1" + }, + "team": { + "id": "00000000-0000-4000-8000-000000000301" + }, + "email": "unregisteredStudentInCourse1@teammates.tmt", + "name": "Unregistered Student In Course1", + "comments": "" } }, "feedbackSessions": { diff --git a/src/main/java/teammates/sqllogic/api/Logic.java b/src/main/java/teammates/sqllogic/api/Logic.java index ed9de2f6b40..ad722629edd 100644 --- a/src/main/java/teammates/sqllogic/api/Logic.java +++ b/src/main/java/teammates/sqllogic/api/Logic.java @@ -99,6 +99,15 @@ public AccountRequest getAccountRequest(String email, String institute) { return accountRequestLogic.getAccountRequest(email, institute); } + /** + * Gets the account request with the associated {@code regkey}. + * + * @return account request with the associated {@code regkey}. + */ + public AccountRequest getAccountRequestByRegistrationKey(String regkey) { + return accountRequestLogic.getAccountRequestByRegistrationKey(regkey); + } + /** * Creates/Resets the account request with the given email and institute * such that it is not registered. diff --git a/src/main/java/teammates/sqllogic/core/AccountRequestsLogic.java b/src/main/java/teammates/sqllogic/core/AccountRequestsLogic.java index 8ae837f09b0..2b0b3100316 100644 --- a/src/main/java/teammates/sqllogic/core/AccountRequestsLogic.java +++ b/src/main/java/teammates/sqllogic/core/AccountRequestsLogic.java @@ -72,6 +72,14 @@ public AccountRequest getAccountRequest(String email, String institute) { return accountRequestDb.getAccountRequest(email, institute); } + /** + * Gets account request associated with the {@code regkey}. + */ + public AccountRequest getAccountRequestByRegistrationKey(String regkey) { + + return accountRequestDb.getAccountRequestByRegistrationKey(regkey); + } + /** * Creates/resets the account request with the given email and institute such that it is not registered. */ diff --git a/src/main/java/teammates/storage/sqlapi/AccountRequestsDb.java b/src/main/java/teammates/storage/sqlapi/AccountRequestsDb.java index a6042e703ad..8c7f3ae38d7 100644 --- a/src/main/java/teammates/storage/sqlapi/AccountRequestsDb.java +++ b/src/main/java/teammates/storage/sqlapi/AccountRequestsDb.java @@ -77,7 +77,7 @@ public AccountRequest getAccountRequest(String email, String institute) { /** * Get AccountRequest by {@code registrationKey} from database. */ - public AccountRequest getAccountRequest(String registrationKey) { + public AccountRequest getAccountRequestByRegistrationKey(String registrationKey) { CriteriaBuilder cb = HibernateUtil.getCriteriaBuilder(); CriteriaQuery cr = cb.createQuery(AccountRequest.class); Root root = cr.from(AccountRequest.class); diff --git a/src/main/java/teammates/ui/webapi/GetCourseJoinStatusAction.java b/src/main/java/teammates/ui/webapi/GetCourseJoinStatusAction.java index 4b45ca24716..e79931dffcb 100644 --- a/src/main/java/teammates/ui/webapi/GetCourseJoinStatusAction.java +++ b/src/main/java/teammates/ui/webapi/GetCourseJoinStatusAction.java @@ -4,12 +4,15 @@ import teammates.common.datatransfer.attributes.InstructorAttributes; import teammates.common.datatransfer.attributes.StudentAttributes; import teammates.common.util.Const; +import teammates.storage.sqlentity.AccountRequest; +import teammates.storage.sqlentity.Instructor; +import teammates.storage.sqlentity.Student; import teammates.ui.output.JoinStatus; /** * Get the join status of a course. */ -class GetCourseJoinStatusAction extends Action { +public class GetCourseJoinStatusAction extends Action { @Override AuthType getMinAuthLevel() { @@ -38,29 +41,49 @@ public JsonResult execute() { } private JsonResult getStudentJoinStatus(String regkey) { - StudentAttributes student = logic.getStudentForRegistrationKey(regkey); - if (student == null) { - throw new EntityNotFoundException("No student with given registration key: " + regkey); + StudentAttributes studentAttributes = logic.getStudentForRegistrationKey(regkey); + + if (studentAttributes != null && !isCourseMigrated(studentAttributes.getCourse())) { + return getJoinStatusResult(studentAttributes.isRegistered()); + } else { + Student student = sqlLogic.getStudentByRegistrationKey(regkey); + + if (student == null) { + throw new EntityNotFoundException("No student with given registration key: " + regkey); + } + return getJoinStatusResult(student.isRegistered()); } - return getJoinStatusResult(student.isRegistered()); } private JsonResult getInstructorJoinStatus(String regkey, boolean isCreatingAccount) { if (isCreatingAccount) { AccountRequestAttributes accountRequest = logic.getAccountRequestForRegistrationKey(regkey); - if (accountRequest == null) { + AccountRequest sqlAccountRequest = sqlLogic.getAccountRequestByRegistrationKey(regkey); + + if (accountRequest == null && sqlAccountRequest == null) { throw new EntityNotFoundException("No account request with given registration key: " + regkey); } - return getJoinStatusResult(accountRequest.getRegisteredAt() != null); + + if (sqlAccountRequest != null) { + return getJoinStatusResult(sqlAccountRequest.getRegisteredAt() != null); + } + if (accountRequest != null) { + return getJoinStatusResult(accountRequest.getRegisteredAt() != null); + } } - InstructorAttributes instructor = logic.getInstructorForRegistrationKey(regkey); + InstructorAttributes instructorAttributes = logic.getInstructorForRegistrationKey(regkey); - if (instructor == null) { - throw new EntityNotFoundException("No instructor with given registration key: " + regkey); - } + if (instructorAttributes != null && !isCourseMigrated(instructorAttributes.getCourseId())) { + return getJoinStatusResult(instructorAttributes.isRegistered()); + } else { + Instructor instructor = sqlLogic.getInstructorByRegistrationKey(regkey); - return getJoinStatusResult(instructor.isRegistered()); + if (instructor == null) { + throw new EntityNotFoundException("No instructor with given registration key: " + regkey); + } + return getJoinStatusResult(instructor.isRegistered()); + } } private JsonResult getJoinStatusResult(boolean hasJoined) {