[#12048] Migrate AdminSearchPageE2ETest (#12838)

* migate admin search e2e * fix e2e test * fix failing tests * fix: add put sql document methods * fix: add migrated check back in * fix: add cleanup method * fix: add search document removal for account request * fix lint and tests * fix: json file formatting * fix: init both searchManagers * fix: add comments * fix: remove notifications field in data file --------- Co-authored-by: Wei Qing <[email protected]>
TEAMMATES · Feb 26, 2024 · 203ec24 · 203ec24
1 parent 21cb04a
commit 203ec24
Show file tree

Hide file tree

Showing 15 changed files with 245 additions and 87 deletions.
diff --git a/src/e2e/java/teammates/e2e/cases/AdminSearchPageE2ETest.java b/src/e2e/java/teammates/e2e/cases/AdminSearchPageE2ETest.java
@@ -2,9 +2,9 @@
 
 import java.time.Instant;
 
+import org.testng.annotations.AfterClass;
 import org.testng.annotations.Test;
 
-import teammates.common.datatransfer.attributes.AccountRequestAttributes;
 import teammates.common.datatransfer.attributes.CourseAttributes;
 import teammates.common.datatransfer.attributes.FeedbackSessionAttributes;
 import teammates.common.datatransfer.attributes.InstructorAttributes;
@@ -13,6 +13,7 @@
 import teammates.common.util.Const;
 import teammates.e2e.pageobjects.AdminSearchPage;
 import teammates.e2e.util.TestProperties;
+import teammates.storage.sqlentity.AccountRequest;
 
 /**
  * SUT: {@link Const.WebPageURIs#ADMIN_SEARCH_PAGE}.
@@ -28,6 +29,9 @@ protected void prepareTestData() {
         testData = loadDataBundle("/AdminSearchPageE2ETest.json");
         removeAndRestoreDataBundle(testData);
         putDocuments(testData);
+        sqlTestData = loadSqlDataBundle("/AdminSearchPageE2ETest_SqlEntities.json");
+        removeAndRestoreSqlDataBundle(sqlTestData);
+        doPutDocumentsSql(sqlTestData);
     }
 
     @Test
@@ -43,7 +47,7 @@ public void testAll() {
         CourseAttributes course = testData.courses.get("typicalCourse1");
         StudentAttributes student = testData.students.get("student1InCourse1");
         InstructorAttributes instructor = testData.instructors.get("instructor1OfCourse1");
-        AccountRequestAttributes accountRequest = testData.accountRequests.get("instructor1OfCourse1");
+        AccountRequest accountRequest = sqlTestData.accountRequests.get("instructor1OfCourse1");
 
         ______TS("Typical case: Search student email");
         String searchContent = student.getEmail();
@@ -131,7 +135,7 @@ public void testAll() {
         assertNull(BACKDOOR.getAccountRequest(accountRequest.getEmail(), accountRequest.getInstitute()).getRegisteredAt());
 
         ______TS("Typical case: Delete account request successful");
-        accountRequest = testData.accountRequests.get("unregisteredInstructor1");
+        accountRequest = sqlTestData.accountRequests.get("unregisteredInstructor1");
         searchContent = accountRequest.getEmail();
         searchPage.clearSearchBox();
         searchPage.inputSearchContent(searchContent);
@@ -186,4 +190,11 @@ private String getExpectedInstructorManageAccountLink(InstructorAttributes instr
                 .toAbsoluteString();
     }
 
+    @AfterClass
+    public void classTeardown() {
+        for (AccountRequest request : sqlTestData.accountRequests.values()) {
+            BACKDOOR.deleteAccountRequest(request.getEmail(), request.getInstitute());
+        }
+    }
+
 }
diff --git a/src/e2e/java/teammates/e2e/cases/BaseE2ETestCase.java b/src/e2e/java/teammates/e2e/cases/BaseE2ETestCase.java
@@ -379,4 +379,15 @@ protected boolean doPutDocuments(DataBundle testData) {
             return false;
         }
     }
+
+    @Override
+    protected boolean doPutDocumentsSql(SqlDataBundle testData) {
+        try {
+            BACKDOOR.putSqlDocuments(testData);
+            return true;
+        } catch (HttpRequestFailedException e) {
+            e.printStackTrace();
+            return false;
+        }
+    }
 }
diff --git a/src/e2e/java/teammates/e2e/pageobjects/AdminSearchPage.java b/src/e2e/java/teammates/e2e/pageobjects/AdminSearchPage.java
@@ -17,6 +17,7 @@
 import teammates.common.datatransfer.attributes.StudentAttributes;
 import teammates.common.util.Const;
 import teammates.common.util.StringHelper;
+import teammates.storage.sqlentity.AccountRequest;
 
 /**
  * Represents the admin home page of the website.
@@ -285,6 +286,22 @@ && removeSpanFromText(columns.get(ACCOUNT_REQUEST_COL_INSTITUTE - 1)
         return null;
     }
 
+    public WebElement getAccountRequestRow(AccountRequest accountRequest) {
+        String email = accountRequest.getEmail();
+        String institute = accountRequest.getInstitute();
+        List<WebElement> rows = browser.driver.findElements(By.cssSelector("#search-table-account-request tbody tr"));
+        for (WebElement row : rows) {
+            List<WebElement> columns = row.findElements(By.tagName("td"));
+            if (removeSpanFromText(columns.get(ACCOUNT_REQUEST_COL_EMAIL - 1)
+                    .getAttribute("innerHTML")).contains(email)
+                    && removeSpanFromText(columns.get(ACCOUNT_REQUEST_COL_INSTITUTE - 1)
+                    .getAttribute("innerHTML")).contains(institute)) {
+                return row;
+            }
+        }
+        return null;
+    }
+
     public String getAccountRequestName(WebElement accountRequestRow) {
         return getColumnText(accountRequestRow, ACCOUNT_REQUEST_COL_NAME);
     }
@@ -317,6 +334,14 @@ public void clickDeleteAccountRequestButton(AccountRequestAttributes accountRequ
         waitForPageToLoad();
     }
 
+    public void clickDeleteAccountRequestButton(AccountRequest accountRequest) {
+        WebElement accountRequestRow = getAccountRequestRow(accountRequest);
+        WebElement deleteButton = accountRequestRow.findElement(By.cssSelector("[id^='delete-account-request-']"));
+        deleteButton.click();
+        waitForConfirmationModalAndClickOk();
+        waitForPageToLoad();
+    }
+
     public void clickResetAccountRequestButton(AccountRequestAttributes accountRequest) {
         WebElement accountRequestRow = getAccountRequestRow(accountRequest);
         WebElement deleteButton = accountRequestRow.findElement(By.cssSelector("[id^='reset-account-request-']"));
@@ -325,6 +350,14 @@ public void clickResetAccountRequestButton(AccountRequestAttributes accountReque
         waitForPageToLoad();
     }
 
+    public void clickResetAccountRequestButton(AccountRequest accountRequest) {
+        WebElement accountRequestRow = getAccountRequestRow(accountRequest);
+        WebElement deleteButton = accountRequestRow.findElement(By.cssSelector("[id^='reset-account-request-']"));
+        deleteButton.click();
+        waitForConfirmationModalAndClickOk();
+        waitForPageToLoad();
+    }
+
     public int getNumExpandedRows(WebElement row) {
         String xpath = "following-sibling::tr[1]/td/ul/li";
         return row.findElements(By.xpath(xpath)).size();
@@ -447,6 +480,25 @@ public void verifyAccountRequestRowContent(AccountRequestAttributes accountReque
         }
     }
 
+    public void verifyAccountRequestRowContent(AccountRequest accountRequest) {
+        WebElement accountRequestRow = getAccountRequestRow(accountRequest);
+        String actualName = getAccountRequestName(accountRequestRow);
+        String actualEmail = getAccountRequestEmail(accountRequestRow);
+        String actualInstitute = getAccountRequestInstitute(accountRequestRow);
+        String actualCreatedAt = getAccountRequestCreatedAt(accountRequestRow);
+        String actualRegisteredAt = getAccountRequestRegisteredAt(accountRequestRow);
+
+        assertEquals(accountRequest.getName(), actualName);
+        assertEquals(accountRequest.getEmail(), actualEmail);
+        assertEquals(accountRequest.getInstitute(), actualInstitute);
+        assertFalse(actualCreatedAt.isBlank());
+        if (accountRequest.getRegisteredAt() == null) {
+            assertEquals("Not Registered Yet", actualRegisteredAt);
+        } else {
+            assertFalse(actualRegisteredAt.isBlank());
+        }
+    }
+
     public void verifyAccountRequestExpandedLinks(AccountRequestAttributes accountRequest) {
         clickExpandAccountRequestLinks();
         WebElement accountRequestRow = getAccountRequestRow(accountRequest);
@@ -455,6 +507,14 @@ public void verifyAccountRequestExpandedLinks(AccountRequestAttributes accountRe
         assertFalse(actualRegistrationLink.isBlank());
     }
 
+    public void verifyAccountRequestExpandedLinks(AccountRequest accountRequest) {
+        clickExpandAccountRequestLinks();
+        WebElement accountRequestRow = getAccountRequestRow(accountRequest);
+        String actualRegistrationLink = getAccountRequestRegistrationLink(accountRequestRow);
+
+        assertFalse(actualRegistrationLink.isBlank());
+    }
+
     public void verifyLinkExpansionButtons(StudentAttributes student,
             InstructorAttributes instructor, AccountRequestAttributes accountRequest) {
         WebElement studentRow = getStudentRow(student);
@@ -492,6 +552,43 @@ public void verifyLinkExpansionButtons(StudentAttributes student,
         assertEquals(numExpandedAccountRequestRows, 0);
     }
 
+    public void verifyLinkExpansionButtons(StudentAttributes student,
+            InstructorAttributes instructor, AccountRequest accountRequest) {
+        WebElement studentRow = getStudentRow(student);
+        WebElement instructorRow = getInstructorRow(instructor);
+        WebElement accountRequestRow = getAccountRequestRow(accountRequest);
+
+        clickExpandStudentLinks();
+        clickExpandInstructorLinks();
+        clickExpandAccountRequestLinks();
+        int numExpandedStudentRows = getNumExpandedRows(studentRow);
+        int numExpandedInstructorRows = getNumExpandedRows(instructorRow);
+        int numExpandedAccountRequestRows = getNumExpandedRows(accountRequestRow);
+        assertNotEquals(numExpandedStudentRows, 0);
+        assertNotEquals(numExpandedInstructorRows, 0);
+        assertNotEquals(numExpandedAccountRequestRows, 0);
+
+        clickCollapseInstructorLinks();
+        numExpandedStudentRows = getNumExpandedRows(studentRow);
+        numExpandedInstructorRows = getNumExpandedRows(instructorRow);
+        numExpandedAccountRequestRows = getNumExpandedRows(accountRequestRow);
+        assertNotEquals(numExpandedStudentRows, 0);
+        assertEquals(numExpandedInstructorRows, 0);
+        assertNotEquals(numExpandedAccountRequestRows, 0);
+
+        clickExpandInstructorLinks();
+        clickCollapseStudentLinks();
+        clickCollapseAccountRequestLinks();
+        waitUntilAnimationFinish();
+
+        numExpandedStudentRows = getNumExpandedRows(studentRow);
+        numExpandedInstructorRows = getNumExpandedRows(instructorRow);
+        numExpandedAccountRequestRows = getNumExpandedRows(accountRequestRow);
+        assertEquals(numExpandedStudentRows, 0);
+        assertNotEquals(numExpandedInstructorRows, 0);
+        assertEquals(numExpandedAccountRequestRows, 0);
+    }
+
     public void verifyRegenerateStudentKey(StudentAttributes student, String originalJoinLink) {
         verifyStatusMessage("Student's key for this course has been successfully regenerated,"
                 + " and the email has been sent.");

diff --git a/src/e2e/resources/data/AdminSearchPageE2ETest.json b/src/e2e/resources/data/AdminSearchPageE2ETest.json
@@ -1,24 +1,4 @@
 {
-  "accounts": {
-    "instructor1OfCourse1": {
-      "googleId": "tm.e2e.ASearch.instr1",
-      "name": "Instructor1 of Course1",
-      "email": "[email protected]",
-      "readNotifications": {}
-    },
-    "instructor2OfCourse1": {
-      "googleId": "tm.e2e.ASearch.instr2",
-      "name": "Instructor2 of Course1",
-      "email": "[email protected]",
-      "readNotifications": {}
-    },
-    "student1InCourse1": {
-      "googleId": "tm.e2e.ASearch.student1",
-      "name": "Student1 in course1",
-      "email": "[email protected]",
-      "readNotifications": {}
-    }
-  },
   "courses": {
     "typicalCourse1": {
       "id": "tm.e2e.ASearch.course1",
@@ -133,27 +113,5 @@
     "studentDeadlines": {},
     "instructorDeadlines": {}
     }
-  },
-  "accountRequests": {
-    "instructor1OfCourse1": {
-      "name": "Instructor1 of Course1",
-      "email": "[email protected]",
-      "institute": "TEAMMATES Test Institute 1",
-      "createdAt": "2011-01-01T00:00:00Z",
-      "registeredAt": "1970-02-14T00:00:00Z"
-    },
-    "instructor2OfCourse1": {
-      "name": "Instructor2 of Course1",
-      "email": "[email protected]",
-      "institute": "TEAMMATES Test Institute 1",
-      "createdAt": "2011-01-01T00:00:00Z",
-      "registeredAt": "1970-02-14T00:00:00Z"
-    },
-    "unregisteredInstructor1": {
-      "name": "Typical Instructor Name",
-      "email": "[email protected]",
-      "institute": "TEAMMATES Test Institute 1",
-      "createdAt": "2011-01-01T00:00:00Z"
-    }
   }
 }
diff --git a/src/e2e/resources/data/AdminSearchPageE2ETest_SqlEntities.json b/src/e2e/resources/data/AdminSearchPageE2ETest_SqlEntities.json
@@ -0,0 +1,44 @@
+{
+  "accounts": {
+    "instructor1OfCourse1": {
+      "id": "00000000-0000-4000-8000-000000000001",
+      "googleId": "tm.e2e.ASearch.instr1",
+      "name": "Instructor1 of Course1",
+      "email": "[email protected]"
+    },
+    "instructor2OfCourse1": {
+      "id": "00000000-0000-4000-8000-000000000002",
+      "googleId": "tm.e2e.ASearch.instr2",
+      "name": "Instructor2 of Course1",
+      "email": "[email protected]"
+    },
+    "student1InCourse1": {
+      "id": "00000000-0000-4000-8000-000000000003",
+      "googleId": "tm.e2e.ASearch.student1",
+      "name": "Student1 in course1",
+      "email": "[email protected]"
+    }
+  },
+  "accountRequests": {
+    "instructor1OfCourse1": {
+      "name": "Instructor1 of Course1",
+      "email": "[email protected]",
+      "institute": "TEAMMATES Test Institute 1",
+      "createdAt": "2011-01-01T00:00:00Z",
+      "registeredAt": "1970-02-14T00:00:00Z"
+    },
+    "instructor2OfCourse1": {
+      "name": "Instructor2 of Course1",
+      "email": "[email protected]",
+      "institute": "TEAMMATES Test Institute 1",
+      "createdAt": "2011-01-01T00:00:00Z",
+      "registeredAt": "1970-02-14T00:00:00Z"
+    },
+    "unregisteredInstructor1": {
+      "name": "Typical Instructor Name",
+      "email": "[email protected]",
+      "institute": "TEAMMATES Test Institute 1",
+      "createdAt": "2011-01-01T00:00:00Z"
+    }
+  }
+}
diff --git a/src/it/java/teammates/it/storage/sqlsearch/InstructorSearchIT.java b/src/it/java/teammates/it/storage/sqlsearch/InstructorSearchIT.java
@@ -123,7 +123,7 @@ public void allTests() throws Exception {
         ______TS("success: search for instructors in whole system; instructors created without searchability unsearchable");
         usersDb.createInstructor(insUniqueDisplayName);
         results = usersDb.searchInstructorsInWholeSystem("\"Instructor of\"");
-        verifySearchResults(results, insInArchivedCourse, insInUnregCourse);
+        verifySearchResults(results, insInArchivedCourse, insInUnregCourse, insUniqueDisplayName);
 
         ______TS("success: search for instructors in whole system; deleting instructor without deleting document:"
                 + "document deleted during search, instructor unsearchable");

diff --git a/src/it/java/teammates/it/test/BaseTestCaseWithSqlDatabaseAccess.java b/src/it/java/teammates/it/test/BaseTestCaseWithSqlDatabaseAccess.java
@@ -85,6 +85,14 @@ protected static void setUpSuite() throws Exception {
             new StudentSearchManager(TestProperties.SEARCH_SERVICE_HOST, true));
 
         // TODO: remove after migration, needed for dual db support
+
+        teammates.storage.search.SearchManagerFactory.registerAccountRequestSearchManager(
+            new teammates.storage.search.AccountRequestSearchManager(TestProperties.SEARCH_SERVICE_HOST, true));
+        teammates.storage.search.SearchManagerFactory.registerInstructorSearchManager(
+            new teammates.storage.search.InstructorSearchManager(TestProperties.SEARCH_SERVICE_HOST, true));
+        teammates.storage.search.SearchManagerFactory.registerStudentSearchManager(
+            new teammates.storage.search.StudentSearchManager(TestProperties.SEARCH_SERVICE_HOST, true));
+
         teammates.logic.core.LogicStarter.initializeDependencies();
         LOCAL_DATASTORE_HELPER.start();
         DatastoreOptions options = LOCAL_DATASTORE_HELPER.getOptions();

diff --git a/src/main/java/teammates/storage/search/SearchManager.java b/src/main/java/teammates/storage/search/SearchManager.java
@@ -227,6 +227,9 @@ List<T> convertDocumentToAttributes(List<SolrDocument> documents) {
             // deleteDocuments(Collections.singletonList(id));
             // continue;
             // }
+            if (attribute == null) {
+                continue;
+            }
             result.add(attribute);
         }
         sortResult(result);

diff --git a/src/main/java/teammates/storage/sqlapi/AccountRequestsDb.java b/src/main/java/teammates/storage/sqlapi/AccountRequestsDb.java
@@ -5,7 +5,9 @@
 
 import java.time.Instant;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
+import java.util.UUID;
 
 import teammates.common.exception.EntityAlreadyExistsException;
 import teammates.common.exception.EntityDoesNotExistException;
@@ -129,6 +131,19 @@ public AccountRequest updateAccountRequest(AccountRequest accountRequest)
     public void deleteAccountRequest(AccountRequest accountRequest) {
         if (accountRequest != null) {
             delete(accountRequest);
+            deleteDocumentByAccountRequestId(accountRequest.getId());
+        }
+    }
+
+    /**
+     * Removes search document for the given account request.
+     */
+    public void deleteDocumentByAccountRequestId(UUID accountRequestId) {
+        if (getSearchManager() != null) {
+            // Solr saves the id with the prefix "java.util.UUID:", so we need to add it here to
+            // identify and delete the document from the index
+            getSearchManager().deleteDocuments(
+                    Collections.singletonList("java.util.UUID:" + accountRequestId.toString()));
         }
     }
 

diff --git a/src/main/java/teammates/storage/sqlsearch/SearchManager.java b/src/main/java/teammates/storage/sqlsearch/SearchManager.java
@@ -223,8 +223,13 @@ List<T> convertDocumentToEntities(List<SolrDocument> documents) {
             if (entity == null) {
                 // search engine out of sync as SearchManager may fail to delete documents
                 // the chance is low and it is generally not a big problem
-                String id = (String) document.getFirstValue("id");
-                deleteDocuments(Collections.singletonList(id));
+
+                // these lines below are commented out as they interfere with the dual db search,
+                // and cause unwanted deletions, please refer to the following PR for more details
+                // [PR](https://github.com/TEAMMATES/teammates/pull/12838)
+
+                // String id = (String) document.getFirstValue("id");
+                // deleteDocuments(Collections.singletonList(id));
                 continue;
             }
             result.add(entity);