Merge branch 'master' into 12048-instructor-course-details-e2e-test-m…

…igration

src/client/java/teammates/client/scripts/sql/DataMigrationEntitiesBaseScriptSql.java

@@ -91,6 +91,12 @@ public DataMigrationEntitiesBaseScriptSql() {
      */
     protected abstract boolean isPreview();
 
+    /**
+     * Sets the criterias used in {@link #isMigrationNeeded(E entity)} on whether migration is needed.
+     * Ran during initialization.
+     */
+    protected abstract void setMigrationCriteria();
+
     /**
      * Checks whether data migration is needed.
      *
@@ -181,6 +187,7 @@ private void migrateWithTrx(Key<E> entityKey) {
     protected void doOperation() {
         log("Running " + getClass().getSimpleName() + "...");
         log("Preview: " + isPreview());
+        setMigrationCriteria();
 
         Cursor cursor = readPositionOfCursorFromFile().orElse(null);
         if (cursor == null) {

src/client/java/teammates/client/scripts/sql/DataMigrationForAccountRequestSql.java

@@ -28,6 +28,14 @@ protected boolean isPreview() {
         return false;
     }
 
+    /*
+     * Sets the migration criteria used in isMigrationNeeded.
+     */
+    @Override
+    protected void setMigrationCriteria() {
+        // No migration criteria currently needed.
+    }
+
     /**
      * Always returns true, as the migration is needed for all entities from
      * Datastore to CloudSQL.

src/client/java/teammates/client/scripts/sql/DataMigrationForNotificationSql.java

@@ -28,6 +28,14 @@ protected boolean isPreview() {
         return false;
     }
 
+    /*
+     * Sets the migration criteria used in isMigrationNeeded.
+    */
+    @Override
+    protected void setMigrationCriteria() {
+        // No migration criteria currently needed.
+    }
+
     @Override
     protected boolean isMigrationNeeded(Notification entity) {
         HibernateUtil.beginTransaction();

src/client/java/teammates/client/scripts/sql/DataMigrationForUsageStatisticsSql.java

@@ -1,18 +1,30 @@
 package teammates.client.scripts.sql;
 
+// CHECKSTYLE.OFF:ImportOrder
+import java.time.Instant;
 import java.util.UUID;
 
 import com.googlecode.objectify.cmd.Query;
 
+import teammates.common.util.HibernateUtil;
+
+import jakarta.persistence.criteria.CriteriaBuilder;
+import jakarta.persistence.criteria.CriteriaQuery;
+import jakarta.persistence.criteria.Root;
 import teammates.storage.sqlentity.UsageStatistics;
+// CHECKSTYLE.ON:ImportOrder
 
 /**
  * Data migration class for usage statistics.
  */
+@SuppressWarnings("PMD")
 public class DataMigrationForUsageStatisticsSql extends
-        DataMigrationEntitiesBaseScriptSql<
-            teammates.storage.entity.UsageStatistics,
-            UsageStatistics> {
+        DataMigrationEntitiesBaseScriptSql<teammates.storage.entity.UsageStatistics, UsageStatistics> {
+
+    // Runs the migration only for newly-created SQL entities since the initial migration.
+    private static final boolean IS_PATCHING_MIGRATION = true;
+
+    private Instant patchingStartTime;
 
     public static void main(String[] args) {
         new DataMigrationForUsageStatisticsSql().doOperationRemotely();
@@ -33,11 +45,43 @@ protected boolean isPreview() {
     }
 
     /**
-     * Always returns true, as the migration is needed for all entities from Datastore to CloudSQL .
+     * Queries for the latest SQL entity created, so that patching will only migrate newly created Datastore entities.
      */
     @Override
+    protected void setMigrationCriteria() {
+        if (!IS_PATCHING_MIGRATION) {
+            return;
+        }
+
+        HibernateUtil.beginTransaction();
+        CriteriaBuilder cb = HibernateUtil.getCriteriaBuilder();
+        CriteriaQuery<Instant> cq = cb.createQuery(Instant.class);
+        Root<UsageStatistics> root = cq.from(UsageStatistics.class);
+        cq.select(cb.greatest(root.<Instant>get("startTime")));
+
+        // If no entity found, Hibernate will return null for Instant instead of throwing NoResultException.
+        patchingStartTime = HibernateUtil.createQuery(cq).getSingleResult();
+        HibernateUtil.commitTransaction();
+
+        if (patchingStartTime == null) {
+            System.out.println(this.getClass().getSimpleName() + " Patching enabled, but unable to find SQL entity");
+            System.exit(1);
+        }
+
+        System.out.println(this.getClass().getSimpleName() + " Patching migration, with time " + patchingStartTime);
+    }
+
+    /**
+     * Always returns true, as the migration is needed for all entities from
+     * Datastore to CloudSQL.
+     */
+    @SuppressWarnings("unused")
+    @Override
     protected boolean isMigrationNeeded(teammates.storage.entity.UsageStatistics entity) {
-        return true;
+        if (patchingStartTime == null) {
+            return true;
+        }
+        return entity.getStartTime().isAfter(patchingStartTime);
     }
 
     @Override

src/client/java/teammates/client/scripts/sql/VerifyAccountAttributes.java

@@ -1,11 +1,21 @@
 package teammates.client.scripts.sql;
 
+// CHECKSTYLE.OFF:ImportOrder
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 
+import jakarta.persistence.TypedQuery;
+import jakarta.persistence.criteria.CriteriaBuilder;
+import jakarta.persistence.criteria.CriteriaQuery;
+import jakarta.persistence.criteria.JoinType;
+import jakarta.persistence.criteria.Order;
+import jakarta.persistence.criteria.Root;
+
+import teammates.common.util.HibernateUtil;
 import teammates.storage.entity.Account;
 import teammates.storage.sqlentity.ReadNotification;
 
@@ -16,6 +26,8 @@
 public class VerifyAccountAttributes
         extends VerifyNonCourseEntityAttributesBaseScript<Account, teammates.storage.sqlentity.Account> {
 
+    private static final String READ_NOTIFICATION_FIELD = "readNotifications";
+
     public VerifyAccountAttributes() {
         super(Account.class,
                 teammates.storage.sqlentity.Account.class);
@@ -46,6 +58,28 @@ public boolean verifyAccountFields(teammates.storage.sqlentity.Account sqlEntity
 
     }
 
+    @Override
+    protected List<teammates.storage.sqlentity.Account> lookupSqlEntitiesByPageNumber(int pageNum) {
+        CriteriaBuilder cb = HibernateUtil.getCriteriaBuilder();
+        CriteriaQuery<teammates.storage.sqlentity.Account> pageQuery = cb.createQuery(sqlEntityClass);
+
+        // sort by id to maintain stable order.
+        Root<teammates.storage.sqlentity.Account> root = pageQuery.from(sqlEntityClass);
+        pageQuery.select(root);
+        List<Order> orderList = new LinkedList<>();
+        orderList.add(cb.asc(root.get("id")));
+        pageQuery.orderBy(orderList);
+
+        // perform query with pagination
+        TypedQuery<teammates.storage.sqlentity.Account> query = HibernateUtil.createQuery(pageQuery);
+        query.setFirstResult(calculateOffset(pageNum));
+        query.setMaxResults(CONST_SQL_FETCH_BASE_SIZE);
+
+        // Fetch read notifications eagerly with one join
+        root.fetch(READ_NOTIFICATION_FIELD, JoinType.LEFT);
+        return query.getResultList();
+    }
+
     // Used for sql data migration
     @Override
     public boolean equals(teammates.storage.sqlentity.Account sqlEntity, Account datastoreEntity) {

src/client/java/teammates/client/scripts/sql/VerifyNonCourseEntityAttributesBaseScript.java

@@ -33,14 +33,19 @@ public abstract class VerifyNonCourseEntityAttributesBaseScript<E extends teamma
      * for optimized batch-fetching.
     */
 
-    private static int constSqlFetchBaseSize = 1000;
+    /**
+     * Batch size to fetch per page.
+     */
+    protected static final int CONST_SQL_FETCH_BASE_SIZE = 1000;
 
     /** Datastore entity class. */
     protected Class<E> datastoreEntityClass;
 
     /** SQL entity class. */
     protected Class<T> sqlEntityClass;
 
+    private long entitiesVerified = 0;
+
     public VerifyNonCourseEntityAttributesBaseScript(
             Class<E> datastoreEntityClass, Class<T> sqlEntityClass) {
         this.datastoreEntityClass = datastoreEntityClass;
@@ -77,8 +82,8 @@ protected Map<String, E> lookupDataStoreEntities(List<String> datastoreEntitiesI
     /**
      * Calculate offset.
      */
-    private int calculateOffset(int pageNum) {
-        return (pageNum - 1) * constSqlFetchBaseSize;
+    protected int calculateOffset(int pageNum) {
+        return (pageNum - 1) * CONST_SQL_FETCH_BASE_SIZE;
     }
 
     /**
@@ -89,17 +94,22 @@ private int getNumPages() {
         CriteriaQuery<Long> countQuery = cb.createQuery(Long.class);
         countQuery.select(cb.count(countQuery.from(sqlEntityClass)));
         long countResults = HibernateUtil.createQuery(countQuery).getSingleResult().longValue();
-        int numPages = (int) (Math.ceil((double) countResults / (double) constSqlFetchBaseSize));
+        int numPages = (int) (Math.ceil((double) countResults / (double) CONST_SQL_FETCH_BASE_SIZE));
         log(String.format("Has %d entities with %d pages", countResults, numPages));
 
         return numPages;
     }
 
-    private List<T> lookupSqlEntitiesByPageNumber(int pageNum) {
+    /**
+     * Sort SQL entities by id in ascending order and return entities on page.
+     * @param pageNum page in a sorted entities tables
+     * @return list of SQL entities on page num
+     */
+    protected List<T> lookupSqlEntitiesByPageNumber(int pageNum) {
         CriteriaBuilder cb = HibernateUtil.getCriteriaBuilder();
         CriteriaQuery<T> pageQuery = cb.createQuery(sqlEntityClass);
 
-        // sort by createdAt to maintain stable order.
+        // sort by id to maintain stable order.
         Root<T> root = pageQuery.from(sqlEntityClass);
         pageQuery.select(root);
         List<Order> orderList = new LinkedList<>();
@@ -109,7 +119,7 @@ private List<T> lookupSqlEntitiesByPageNumber(int pageNum) {
         // perform query with pagination
         TypedQuery<T> query = HibernateUtil.createQuery(pageQuery);
         query.setFirstResult(calculateOffset(pageNum));
-        query.setMaxResults(constSqlFetchBaseSize);
+        query.setMaxResults(CONST_SQL_FETCH_BASE_SIZE);
 
         return query.getResultList();
     }
@@ -149,15 +159,18 @@ protected List<Map.Entry<T, E>> checkAllEntitiesForFailures() {
                     + (endTimeForDatastore - startTimeForDatastore) + " milliseconds");
 
             long startTimeForEquals = System.currentTimeMillis();
+            entitiesVerified += sqlEntities.size();
             for (T sqlEntity : sqlEntities) {
                 E datastoreEntity = datastoreEntities.get(generateID(sqlEntity));
                 if (datastoreEntity == null) {
+                    entitiesVerified -= 1;
                     failures.add(new AbstractMap.SimpleEntry<T, E>(sqlEntity, null));
                     continue;
                 }
 
                 boolean isEqual = equals(sqlEntity, datastoreEntity);
                 if (!isEqual) {
+                    entitiesVerified -= 1;
                     failures.add(new AbstractMap.SimpleEntry<T, E>(sqlEntity, datastoreEntity));
                     continue;
                 }
@@ -175,6 +188,7 @@ protected List<Map.Entry<T, E>> checkAllEntitiesForFailures() {
     protected void runCheckAllEntities(Class<T> sqlEntityClass,
             Class<E> datastoreEntityClass) {
         HibernateUtil.beginTransaction();
+        long checkStartTime = System.currentTimeMillis();
         List<Map.Entry<T, E>> failedEntities = checkAllEntitiesForFailures();
 
         System.out.println("========================================");
@@ -186,6 +200,10 @@ protected void runCheckAllEntities(Class<T> sqlEntityClass,
         } else {
             log("No errors detected");
         }
+
+        long checkEndTime = System.currentTimeMillis();
+        log("Entity took " + (checkEndTime - checkStartTime) + " milliseconds to verify");
+        log("Verified " + entitiesVerified + " SQL entities successfully");
         HibernateUtil.commitTransaction();
     }
 

src/e2e/java/teammates/e2e/cases/sql/InstructorNotificationsPageE2ETest.java

@@ -0,0 +1,78 @@
+package teammates.e2e.cases.sql;
+
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.Test;
+
+import teammates.common.util.AppUrl;
+import teammates.common.util.Const;
+import teammates.e2e.pageobjects.InstructorNotificationsPage;
+import teammates.storage.sqlentity.Account;
+import teammates.storage.sqlentity.Notification;
+import teammates.ui.output.AccountData;
+
+/**
+ * SUT: {@link Const.WebPageURIs#INSTRUCTOR_NOTIFICATIONS_PAGE}.
+ */
+public class InstructorNotificationsPageE2ETest extends BaseE2ETestCase {
+
+    @Override
+    protected void prepareTestData() {
+        testData = loadSqlDataBundle("/InstructorNotificationsPageE2ESqlTest.json");
+        removeAndRestoreDataBundle(testData);
+    }
+
+    @Test
+    @Override
+    protected void testAll() {
+        Account account = testData.accounts.get("INotifs.instr");
+        AppUrl notificationsPageUrl = createFrontendUrl(Const.WebPageURIs.INSTRUCTOR_NOTIFICATIONS_PAGE);
+        InstructorNotificationsPage notificationsPage = loginToPage(notificationsPageUrl, InstructorNotificationsPage.class,
+                account.getGoogleId());
+
+        ______TS("verify that only active notifications with correct target user are shown");
+        Notification[] notShownNotifications = {
+                testData.notifications.get("notification2"),
+                testData.notifications.get("expiredNotification1"),
+        };
+        Notification[] shownNotifications = {
+                testData.notifications.get("notification1"),
+                testData.notifications.get("notification3"),
+                testData.notifications.get("notification4"),
+        };
+
+        Notification[] readNotifications = {
+                testData.notifications.get("notification4"),
+        };
+
+        Set<String> readNotificationsIds = Stream.of(readNotifications)
+                .map(readNotification -> readNotification.getId().toString())
+                .collect(Collectors.toSet());
+
+        notificationsPage.verifyNotShownNotifications(notShownNotifications);
+        notificationsPage.verifyShownNotifications(shownNotifications, readNotificationsIds);
+
+        ______TS("mark notification as read");
+        Notification notificationToMarkAsRead = testData.notifications.get("notification3");
+        notificationsPage.markNotificationAsRead(notificationToMarkAsRead);
+        notificationsPage.verifyStatusMessage("Notification marked as read.");
+
+        // Verify that account's readNotifications attribute is updated
+        AccountData accountFromDb = BACKDOOR.getAccountData(account.getGoogleId());
+        assertTrue(accountFromDb.getReadNotifications().containsKey(notificationToMarkAsRead.getId().toString()));
+
+        ______TS("notification banner is not visible");
+        assertFalse(notificationsPage.isBannerVisible());
+    }
+
+    @AfterClass
+    public void classTeardown() {
+        for (Notification notification : testData.notifications.values()) {
+            BACKDOOR.deleteNotification(notification.getId());
+        }
+    }
+
+}