[#11878] Fix Account Request Update Search Indexing (#12984)

* update account request indexing

* add methods to test access control

* refactoring for transactions

src/it/java/teammates/it/ui/webapi/AccountRequestSearchIndexingWorkerActionIT.java

@@ -1,6 +1,7 @@
 package teammates.it.ui.webapi;
 
 import java.util.List;
+import java.util.UUID;
 
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
@@ -46,6 +47,7 @@ public void testExecute() throws Exception {
         }
 
         AccountRequest accountRequest = typicalBundle.accountRequests.get("instructor1");
+        UUID accountRequestId = accountRequest.getId();
 
         ______TS("account request not yet indexed should not be searchable");
 
@@ -56,8 +58,7 @@ public void testExecute() throws Exception {
         ______TS("account request indexed should be searchable");
 
         String[] submissionParams = new String[] {
-                ParamsNames.INSTRUCTOR_EMAIL, accountRequest.getEmail(),
-                ParamsNames.INSTRUCTOR_INSTITUTION, accountRequest.getInstitute(),
+                ParamsNames.ACCOUNT_REQUEST_ID, accountRequestId.toString(),
         };
 
         AccountRequestSearchIndexingWorkerAction action = getAction(submissionParams);

src/it/java/teammates/it/ui/webapi/BaseActionIT.java

@@ -22,6 +22,7 @@
 import teammates.common.util.Config;
 import teammates.common.util.Const;
 import teammates.common.util.EmailWrapper;
+import teammates.common.util.HibernateUtil;
 import teammates.common.util.JsonUtils;
 import teammates.it.test.BaseTestCaseWithSqlDatabaseAccess;
 import teammates.logic.api.MockEmailSender;
@@ -169,6 +170,14 @@ protected void loginAsAdmin() {
         assertTrue(user.isAdmin);
     }
 
+    /**
+     * Logs in the user to the test environment as an admin.
+     */
+    protected void loginAsAdminWithTransaction() {
+        UserInfo user = mockUserProvision.loginAsAdminWithTransaction(Config.APP_ADMINS.get(0));
+        assertTrue(user.isAdmin);
+    }
+
     /**
      * Logs in the user to the test environment as an unregistered user
      * (without any right).
@@ -180,6 +189,17 @@ protected void loginAsUnregistered(String userId) {
         assertFalse(user.isAdmin);
     }
 
+    /**
+     * Logs in the user to the test environment as an unregistered user
+     * (without any right).
+     */
+    protected void loginAsUnregisteredWithTransaction(String userId) {
+        UserInfo user = mockUserProvision.loginUserWithTransaction(userId);
+        assertFalse(user.isStudent);
+        assertFalse(user.isInstructor);
+        assertFalse(user.isAdmin);
+    }
+
     /**
      * Logs in the user to the test environment as an instructor
      * (without admin rights or student rights).
@@ -191,6 +211,17 @@ protected void loginAsInstructor(String userId) {
         assertFalse(user.isAdmin);
     }
 
+    /**
+     * Logs in the user to the test environment as an instructor
+     * (without admin rights or student rights).
+     */
+    protected void loginAsInstructorWithTransaction(String userId) {
+        UserInfo user = mockUserProvision.loginUserWithTransaction(userId);
+        assertFalse(user.isStudent);
+        assertTrue(user.isInstructor);
+        assertFalse(user.isAdmin);
+    }
+
     /**
      * Logs in the user to the test environment as a student
      * (without admin rights or instructor rights).
@@ -202,6 +233,17 @@ protected void loginAsStudent(String userId) {
         assertFalse(user.isAdmin);
     }
 
+    /**
+     * Logs in the user to the test environment as a student
+     * (without admin rights or instructor rights).
+     */
+    protected void loginAsStudentWithTransaction(String userId) {
+        UserInfo user = mockUserProvision.loginUserWithTransaction(userId);
+        assertTrue(user.isStudent);
+        assertFalse(user.isInstructor);
+        assertFalse(user.isAdmin);
+    }
+
     /**
      * Logs in the user to the test environment as a student-instructor (without
      * admin rights).
@@ -267,6 +309,24 @@ void verifyOnlyAdminCanAccess(Course course, String... params)
         verifyAccessibleForAdmin(params);
     }
 
+    void verifyOnlyAdminCanAccessWithTransaction(String... params)
+            throws InvalidParametersException, EntityAlreadyExistsException {
+        HibernateUtil.beginTransaction();
+        Course course = getTypicalCourse();
+        course = logic.createCourse(course);
+        HibernateUtil.commitTransaction();
+
+        verifyInaccessibleWithoutLogin(params);
+        verifyInaccessibleForUnregisteredUsersWithTransaction(params);
+        verifyInaccessibleForStudentsWithTransaction(course, params);
+        verifyInaccessibleForInstructorsWithTransaction(course, params);
+        verifyAccessibleForAdminWithTransaction(params);
+
+        HibernateUtil.beginTransaction();
+        logic.deleteCourseCascade(course.getId());
+        HibernateUtil.commitTransaction();
+    }
+
     void verifyOnlyInstructorsCanAccess(Course course, String... params)
             throws InvalidParametersException, EntityAlreadyExistsException {
         verifyInaccessibleWithoutLogin(params);
@@ -329,13 +389,28 @@ void verifyInaccessibleForUnregisteredUsers(String... params) {
         verifyCannotAccess(params);
     }
 
+    void verifyInaccessibleForUnregisteredUsersWithTransaction(String... params) {
+        ______TS("Non-registered users cannot access");
+
+        String unregUserId = "unreg.user";
+        loginAsUnregisteredWithTransaction(unregUserId);
+        verifyCannotAccess(params);
+    }
+
     void verifyAccessibleForAdmin(String... params) {
         ______TS("Admin can access");
 
         loginAsAdmin();
         verifyCanAccess(params);
     }
 
+    void verifyAccessibleForAdminWithTransaction(String... params) {
+        ______TS("Admin can access");
+
+        loginAsAdminWithTransaction();
+        verifyCanAccess(params);
+    }
+
     void verifyInaccessibleForAdmin(String... params) {
         ______TS("Admin cannot access");
 
@@ -353,6 +428,21 @@ void verifyInaccessibleForStudents(Course course, String... params)
 
     }
 
+    void verifyInaccessibleForStudentsWithTransaction(Course course, String... params)
+            throws InvalidParametersException, EntityAlreadyExistsException {
+        ______TS("Students cannot access");
+        HibernateUtil.beginTransaction();
+        Student student = createTypicalStudent(course, "[email protected]");
+        HibernateUtil.commitTransaction();
+
+        loginAsStudentWithTransaction(student.getAccount().getGoogleId());
+        verifyCannotAccess(params);
+
+        HibernateUtil.beginTransaction();
+        logic.deleteAccountCascade(student.getAccount().getGoogleId());
+        HibernateUtil.commitTransaction();
+    }
+
     void verifyInaccessibleForInstructors(Course course, String... params)
             throws InvalidParametersException, EntityAlreadyExistsException {
         ______TS("Instructors cannot access");
@@ -363,6 +453,21 @@ void verifyInaccessibleForInstructors(Course course, String... params)
 
     }
 
+    void verifyInaccessibleForInstructorsWithTransaction(Course course, String... params)
+            throws InvalidParametersException, EntityAlreadyExistsException {
+        ______TS("Instructors cannot access");
+        HibernateUtil.beginTransaction();
+        Instructor instructor = createTypicalInstructor(course, "[email protected]");
+        HibernateUtil.commitTransaction();
+
+        loginAsInstructorWithTransaction(instructor.getAccount().getGoogleId());
+        verifyCannotAccess(params);
+
+        HibernateUtil.beginTransaction();
+        logic.deleteAccountCascade(instructor.getAccount().getGoogleId());
+        HibernateUtil.commitTransaction();
+    }
+
     void verifyAccessibleForAdminToMasqueradeAsInstructor(
             Instructor instructor, String[] submissionParams) {
         ______TS("admin can access");
@@ -738,5 +843,4 @@ private Student createTypicalStudent(Course course, String email)
         }
         return student;
     }
-
 }

src/it/java/teammates/it/ui/webapi/UpdateAccountRequestActionIT.java

@@ -1,7 +1,9 @@
 package teammates.it.ui.webapi;
 
+import java.util.List;
 import java.util.UUID;
 
+import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
@@ -13,7 +15,6 @@
 import teammates.common.util.HibernateUtil;
 import teammates.common.util.StringHelperExtension;
 import teammates.storage.sqlentity.AccountRequest;
-import teammates.storage.sqlentity.Course;
 import teammates.ui.output.AccountRequestData;
 import teammates.ui.request.AccountRequestUpdateRequest;
 import teammates.ui.request.InvalidHttpRequestBodyException;
@@ -30,9 +31,7 @@ public class UpdateAccountRequestActionIT extends BaseActionIT<UpdateAccountRequ
     @Override
     @BeforeMethod
     protected void setUp() throws Exception {
-        super.setUp();
-        persistDataBundle(typicalBundle);
-        HibernateUtil.flushSession();
+        // no need to call super.setUp() because the action handles its own transactions
     }
 
     @Override
@@ -49,8 +48,8 @@ protected String getRequestMethod() {
     @Test
     public void testExecute() throws Exception {
         ______TS("edit fields of an account request");
-        AccountRequest accountRequest = typicalBundle.accountRequests.get("unregisteredInstructor1");
-        accountRequest.setStatus(AccountRequestStatus.PENDING);
+        AccountRequest accountRequest = logic.createAccountRequestWithTransaction("name", "[email protected]",
+                "institute", AccountRequestStatus.PENDING, "comments");
         UUID id = accountRequest.getId();
         String name = "newName";
         String email = "[email protected]";
@@ -75,8 +74,8 @@ public void testExecute() throws Exception {
         verifyNoEmailsSent();
 
         ______TS("approve a pending account request");
-        accountRequest = typicalBundle.accountRequests.get("unregisteredInstructor2");
-        accountRequest.setStatus(AccountRequestStatus.PENDING);
+        accountRequest = logic.createAccountRequestWithTransaction("name", "[email protected]",
+                "institute", AccountRequestStatus.PENDING, "comments");
         requestBody = new AccountRequestUpdateRequest(accountRequest.getName(), accountRequest.getEmail(),
                 accountRequest.getInstitute(), AccountRequestStatus.APPROVED, accountRequest.getComments());
         params = new String[] {Const.ParamsNames.ACCOUNT_REQUEST_ID, accountRequest.getId().toString()};
@@ -92,7 +91,8 @@ public void testExecute() throws Exception {
         verifyNumberOfEmailsSent(1);
 
         ______TS("already registered account request has no email sent when approved");
-        accountRequest = typicalBundle.accountRequests.get("instructor2");
+        accountRequest = logic.createAccountRequestWithTransaction("name", "[email protected]",
+                "institute", AccountRequestStatus.REGISTERED, "comments");
         requestBody = new AccountRequestUpdateRequest(name, email, institute, AccountRequestStatus.APPROVED, comments);
         params = new String[] {Const.ParamsNames.ACCOUNT_REQUEST_ID, accountRequest.getId().toString()};
 
@@ -127,7 +127,8 @@ public void testExecute() throws Exception {
         assertEquals("Invalid UUID string: invalid", ihpe.getMessage());
 
         ______TS("invalid email");
-        accountRequest = typicalBundle.accountRequests.get("unregisteredInstructor1");
+        accountRequest = logic.createAccountRequestWithTransaction("name", "[email protected]",
+                "institute", AccountRequestStatus.PENDING, "comments");
         id = accountRequest.getId();
         email = "newEmail";
         status = accountRequest.getStatus();
@@ -217,7 +218,17 @@ public void testExecute() throws Exception {
     @Override
     @Test
     protected void testAccessControl() throws InvalidParametersException, EntityAlreadyExistsException {
-        Course course = typicalBundle.courses.get("course1");
-        verifyOnlyAdminCanAccess(course);
+        verifyOnlyAdminCanAccessWithTransaction();
+    }
+
+    @Override
+    @AfterMethod
+    protected void tearDown() {
+        HibernateUtil.beginTransaction();
+        List<AccountRequest> accountRequests = logic.getAllAccountRequests();
+        for (AccountRequest ar : accountRequests) {
+            logic.deleteAccountRequest(ar.getEmail(), ar.getInstitute());
+        }
+        HibernateUtil.commitTransaction();
     }
 }

src/main/java/teammates/logic/api/TaskQueuer.java

@@ -218,15 +218,13 @@ public void scheduleInstructorForSearchIndexing(String courseId, String email) {
     }
 
     /**
-     * Schedules for the search indexing of the account request identified by {@code email} and {@code institute}.
+     * Schedules for the search indexing of the account request identified by {@code id}.
      *
-     * @param email the email associated with the account request
-     * @param institute the institute associated with the account request
+     * @param id the id associated with the account request
      */
-    public void scheduleAccountRequestForSearchIndexing(String email, String institute) {
+    public void scheduleAccountRequestForSearchIndexing(String id) {
         Map<String, String> paramMap = new HashMap<>();
-        paramMap.put(ParamsNames.INSTRUCTOR_EMAIL, email);
-        paramMap.put(ParamsNames.INSTRUCTOR_INSTITUTION, institute);
+        paramMap.put(ParamsNames.ACCOUNT_REQUEST_ID, id);
 
         addTask(TaskQueue.SEARCH_INDEXING_QUEUE_NAME, TaskQueue.ACCOUNT_REQUEST_SEARCH_INDEXING_WORKER_URL,
                 paramMap, null);

src/main/java/teammates/sqllogic/api/Logic.java

@@ -113,6 +113,15 @@ public AccountRequest getAccountRequest(String email, String institute) {
         return accountRequestLogic.getAccountRequest(email, institute);
     }
 
+    /**
+     * Gets the account request with the given {@code id}.
+     *
+     * @return account request with the given {@code id}.
+     */
+    public AccountRequest getAccountRequestWithTransaction(UUID id) {
+        return accountRequestLogic.getAccountRequestWithTransaction(id);
+    }
+
     /**
      * Creates a or gets an account request.
      *
@@ -145,6 +154,16 @@ public AccountRequest updateAccountRequest(AccountRequest accountRequest)
         return accountRequestLogic.updateAccountRequest(accountRequest);
     }
 
+    /**
+     * Updates the given account request.
+     *
+     * @return the updated account request.
+     */
+    public AccountRequest updateAccountRequestWithTransaction(AccountRequest accountRequest)
+            throws InvalidParametersException, EntityDoesNotExistException {
+        return accountRequestLogic.updateAccountRequestWithTransaction(accountRequest);
+    }
+
     /**
      * Creates/Resets the account request with the given email and institute
      * such that it is not registered.
@@ -178,6 +197,13 @@ public List<AccountRequest> getPendingAccountRequests() {
         return accountRequestLogic.getPendingAccountRequests();
     }
 
+    /**
+     * Gets all pending account requests.
+     */
+    public List<AccountRequest> getAllAccountRequests() {
+        return accountRequestLogic.getAllAccountRequests();
+    }
+
     /**
      * Gets an account.
      */