From affcc4638c15450877a45fb2367eeb36d17b8218 Mon Sep 17 00:00:00 2001 From: Cedric Ong <67156011+cedricongjh@users.noreply.github.com> Date: Sat, 24 Feb 2024 15:26:29 +0800 Subject: [PATCH] [#12048] Migrate student notification page e2e test (#12773) * Persist ReadNotifications in databundle * Add remove or replace sql databundle methods into base test case * Remove unnecessary checking of account migrated * Migrate StudentNotificationPageE2ETest * Fix lint issues * Return sqldatabundle for tests * Revert changes to NotificationPage to use notification id instead * Add assertion for readNotifications from account * fix architectureTest --------- Co-authored-by: Wei Qing <48304907+weiquu@users.noreply.github.com> --- .../teammates/e2e/cases/BaseE2ETestCase.java | 16 ++++ .../StudentNotificationsPageE2ETest.java | 54 ++++++++------ .../pageobjects/UserNotificationsPage.java | 67 +++++++++++++++++ .../data/StudentNotificationsPageE2ETest.json | 67 ----------------- ...tNotificationsPageE2ETest_SqlEntities.json | 73 +++++++++++++++++++ .../it/sqllogic/core/DataBundleLogicIT.java | 3 +- .../BaseTestCaseWithSqlDatabaseAccess.java | 3 +- .../java/teammates/sqllogic/api/Logic.java | 2 +- .../sqllogic/core/DataBundleLogic.java | 11 ++- .../java/teammates/ui/output/AccountData.java | 2 +- .../ui/webapi/CreateAccountAction.java | 4 +- .../teammates/ui/webapi/GetAccountAction.java | 20 ++--- .../webapi/MarkNotificationAsReadAction.java | 6 -- .../ui/webapi/PutSqlDataBundleAction.java | 3 + .../architecture/ArchitectureTest.java | 8 +- .../java/teammates/test/AbstractBackDoor.java | 34 +++++++-- .../test/BaseTestCaseWithDatabaseAccess.java | 16 ++++ .../BaseTestCaseWithLocalDatabaseAccess.java | 15 ++++ 18 files changed, 280 insertions(+), 124 deletions(-) create mode 100644 src/e2e/resources/data/StudentNotificationsPageE2ETest_SqlEntities.json diff --git a/src/e2e/java/teammates/e2e/cases/BaseE2ETestCase.java b/src/e2e/java/teammates/e2e/cases/BaseE2ETestCase.java index ce01e80bcb8..7dedf2d51d9 100644 --- a/src/e2e/java/teammates/e2e/cases/BaseE2ETestCase.java +++ b/src/e2e/java/teammates/e2e/cases/BaseE2ETestCase.java @@ -10,6 +10,7 @@ import org.testng.annotations.BeforeClass; import teammates.common.datatransfer.DataBundle; +import teammates.common.datatransfer.SqlDataBundle; import teammates.common.datatransfer.attributes.AccountAttributes; import teammates.common.datatransfer.attributes.AccountRequestAttributes; import teammates.common.datatransfer.attributes.CourseAttributes; @@ -53,6 +54,11 @@ public abstract class BaseE2ETestCase extends BaseTestCaseWithDatabaseAccess { */ protected DataBundle testData; + /** + * Sql Data to be used in the test. + */ + protected SqlDataBundle sqlTestData; + private Browser browser; @BeforeClass @@ -353,6 +359,16 @@ protected boolean doRemoveAndRestoreDataBundle(DataBundle testData) { } } + @Override + protected SqlDataBundle doRemoveAndRestoreSqlDataBundle(SqlDataBundle testData) { + try { + return BACKDOOR.removeAndRestoreSqlDataBundle(testData); + } catch (HttpRequestFailedException e) { + e.printStackTrace(); + return null; + } + } + @Override protected boolean doPutDocuments(DataBundle testData) { try { diff --git a/src/e2e/java/teammates/e2e/cases/StudentNotificationsPageE2ETest.java b/src/e2e/java/teammates/e2e/cases/StudentNotificationsPageE2ETest.java index f4a52e00f2e..e41e3bda36a 100644 --- a/src/e2e/java/teammates/e2e/cases/StudentNotificationsPageE2ETest.java +++ b/src/e2e/java/teammates/e2e/cases/StudentNotificationsPageE2ETest.java @@ -1,17 +1,18 @@ package teammates.e2e.cases; -import java.time.Instant; -import java.util.HashMap; -import java.util.Map; +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.datatransfer.attributes.AccountAttributes; -import teammates.common.datatransfer.attributes.NotificationAttributes; import teammates.common.util.AppUrl; import teammates.common.util.Const; import teammates.e2e.pageobjects.StudentNotificationsPage; +import teammates.storage.sqlentity.Account; +import teammates.storage.sqlentity.Notification; +import teammates.ui.output.AccountData; /** * SUT: {@link Const.WebPageURIs#STUDENT_NOTIFICATIONS_PAGE}. @@ -22,43 +23,48 @@ public class StudentNotificationsPageE2ETest extends BaseE2ETestCase { protected void prepareTestData() { testData = loadDataBundle("/StudentNotificationsPageE2ETest.json"); removeAndRestoreDataBundle(testData); + sqlTestData = removeAndRestoreSqlDataBundle( + loadSqlDataBundle("/StudentNotificationsPageE2ETest_SqlEntities.json")); } @Test @Override public void testAll() { - AccountAttributes account = testData.accounts.get("SNotifs.student"); + Account account = sqlTestData.accounts.get("SNotifs.student"); AppUrl notificationsPageUrl = createFrontendUrl(Const.WebPageURIs.STUDENT_NOTIFICATIONS_PAGE); StudentNotificationsPage notificationsPage = loginToPage(notificationsPageUrl, StudentNotificationsPage.class, account.getGoogleId()); ______TS("verify that only active notifications with correct target user are shown"); - NotificationAttributes[] notShownNotifications = { - testData.notifications.get("notification3"), - testData.notifications.get("expiredNotification1"), + Notification[] notShownNotifications = { + sqlTestData.notifications.get("notification3"), + sqlTestData.notifications.get("expiredNotification1"), }; - NotificationAttributes[] shownNotifications = { - testData.notifications.get("notification1"), - testData.notifications.get("notification2"), - testData.notifications.get("notification4"), + Notification[] shownNotifications = { + sqlTestData.notifications.get("notification1"), + sqlTestData.notifications.get("notification2"), + sqlTestData.notifications.get("notification4"), }; + Notification[] readNotifications = { + sqlTestData.notifications.get("notification4"), + }; + + Set readNotificationsIds = Stream.of(readNotifications) + .map(readNotification -> readNotification.getId().toString()) + .collect(Collectors.toSet()); + notificationsPage.verifyNotShownNotifications(notShownNotifications); - notificationsPage.verifyShownNotifications(shownNotifications, account.getReadNotifications().keySet()); + notificationsPage.verifyShownNotifications(shownNotifications, readNotificationsIds); ______TS("mark notification as read"); - NotificationAttributes notificationToMarkAsRead = testData.notifications.get("notification2"); + Notification notificationToMarkAsRead = sqlTestData.notifications.get("notification2"); notificationsPage.markNotificationAsRead(notificationToMarkAsRead); notificationsPage.verifyStatusMessage("Notification marked as read."); // Verify that account's readNotifications attribute is updated - Map readNotifications = new HashMap<>(); - readNotifications.put(notificationToMarkAsRead.getNotificationId(), notificationToMarkAsRead.getEndTime()); - readNotifications.putAll(account.getReadNotifications()); - account.setReadNotifications(readNotifications); - verifyPresentInDatabase(account); - - notificationsPage.verifyNotificationTab(notificationToMarkAsRead, account.getReadNotifications().keySet()); + AccountData accountFromDb = BACKDOOR.getAccountData(account.getGoogleId()); + assertTrue(accountFromDb.getReadNotifications().containsKey(notificationToMarkAsRead.getId().toString())); ______TS("notification banner is not visible"); assertFalse(notificationsPage.isBannerVisible()); @@ -66,8 +72,8 @@ public void testAll() { @AfterClass public void classTeardown() { - for (NotificationAttributes notification : testData.notifications.values()) { - BACKDOOR.deleteNotification(notification.getNotificationId()); + for (Notification notification : sqlTestData.notifications.values()) { + BACKDOOR.deleteNotification(notification.getId()); } } diff --git a/src/e2e/java/teammates/e2e/pageobjects/UserNotificationsPage.java b/src/e2e/java/teammates/e2e/pageobjects/UserNotificationsPage.java index e63a88d62b9..1460a699c52 100644 --- a/src/e2e/java/teammates/e2e/pageobjects/UserNotificationsPage.java +++ b/src/e2e/java/teammates/e2e/pageobjects/UserNotificationsPage.java @@ -15,6 +15,7 @@ import teammates.common.datatransfer.NotificationStyle; import teammates.common.datatransfer.attributes.NotificationAttributes; +import teammates.storage.sqlentity.Notification; /** * Page Object Model for user notifications page. @@ -44,6 +45,14 @@ public void verifyNotShownNotifications(NotificationAttributes[] notifications) } } + public void verifyNotShownNotifications(Notification[] notifications) { + List shownNotificationIds = notificationTabs.findElements(By.className("card")) + .stream().map(e -> e.getAttribute("id")).collect(Collectors.toList()); + for (Notification notification : notifications) { + assertFalse(shownNotificationIds.contains(notification.getId().toString())); + } + } + public void verifyShownNotifications(NotificationAttributes[] notifications, Set readNotificationIds) { // Only validates that the preset notifications are present instead of checking every notification // This is because the page will display all active notifications in the database, which is not predictable @@ -52,6 +61,14 @@ public void verifyShownNotifications(NotificationAttributes[] notifications, Set } } + public void verifyShownNotifications(Notification[] notifications, Set readNotificationIds) { + // Only validates that the preset notifications are present instead of checking every notification + // This is because the page will display all active notifications in the database, which is not predictable + for (Notification notification : notifications) { + verifyNotificationTab(notification, readNotificationIds); + } + } + public void verifyNotificationTab(NotificationAttributes notification, Set readNotificationIds) { boolean isRead = readNotificationIds.contains(notification.getNotificationId()); WebElement notificationTab = notificationTabs.findElement(By.id(notification.getNotificationId())); @@ -91,12 +108,57 @@ public void verifyNotificationTab(NotificationAttributes notification, Set readNotificationIds) { + boolean isRead = readNotificationIds.contains(notification.getId().toString()); + WebElement notificationTab = notificationTabs.findElement(By.id(notification.getId().toString())); + + // Check text and style of notification header + WebElement cardHeader = notificationTab.findElement(By.className("card-header")); + assertEquals(getHeaderText(notification), cardHeader.getText()); + assertTrue(cardHeader.getAttribute("class").contains(getHeaderClass(notification.getStyle()))); + + // Checks if tab is open if notification is unread, and closed if notification is read + String chevronClass = notificationTab.findElement(By.tagName("i")).getAttribute("class"); + if (isRead) { + assertTrue(chevronClass.contains("fa-chevron-down")); + // Open tab if notification is unread + click(cardHeader); + waitForPageToLoad(); + } else { + assertTrue(chevronClass.contains("fa-chevron-up")); + } + + // Check notification message + WebElement notifMessage = notificationTab.findElement(By.className("notification-message")); + assertEquals(notification.getMessage(), notifMessage.getAttribute("innerHTML")); + + List markAsReadBtnList = notificationTab.findElements(By.className("btn-mark-as-read")); + + if (isRead) { + // Check that mark as read button cannot be found if notification is read + assertEquals(0, markAsReadBtnList.size()); + + // Close tab if notification is read + click(cardHeader); + waitForPageToLoad(); + } else { + // Check style of mark as read button if notification is unread + assertTrue(markAsReadBtnList.get(0).getAttribute("class").contains(getButtonClass(notification.getStyle()))); + } + } + public void markNotificationAsRead(NotificationAttributes notification) { WebElement notificationTab = notificationTabs.findElement(By.id(notification.getNotificationId())); click(notificationTab.findElement(By.className("btn-mark-as-read"))); waitForPageToLoad(true); } + public void markNotificationAsRead(Notification notification) { + WebElement notificationTab = notificationTabs.findElement(By.id(notification.getId().toString())); + click(notificationTab.findElement(By.className("btn-mark-as-read"))); + waitForPageToLoad(true); + } + private String getTimezone() { return notificationsTimezone.getText().replace("All dates are displayed in ", "").replace(" time.", ""); } @@ -106,6 +168,11 @@ private String getHeaderText(NotificationAttributes notification) { getHeaderDateString(notification.getStartTime()), getHeaderDateString(notification.getEndTime())); } + private String getHeaderText(Notification notification) { + return String.format("%s [%s - %s]", notification.getTitle(), + getHeaderDateString(notification.getStartTime()), getHeaderDateString(notification.getEndTime())); + } + private String getHeaderDateString(Instant date) { return getDisplayedDateTime(date, getTimezone(), "dd MMM yyyy"); } diff --git a/src/e2e/resources/data/StudentNotificationsPageE2ETest.json b/src/e2e/resources/data/StudentNotificationsPageE2ETest.json index 1f8ab61347e..467b994bdda 100644 --- a/src/e2e/resources/data/StudentNotificationsPageE2ETest.json +++ b/src/e2e/resources/data/StudentNotificationsPageE2ETest.json @@ -1,14 +1,4 @@ { - "accounts": { - "SNotifs.student": { - "googleId": "tm.e2e.SNotifs.student", - "name": "Alice B", - "email": "SNotifs.student@gmail.tmt", - "readNotifications": { - "notification4": "2099-04-04T00:00:00Z" - } - } - }, "courses": { "typicalCourse1": { "id": "tm.e2e.SNotifs.course1", @@ -27,62 +17,5 @@ "team": "Team 1'\"", "section": "None" } - }, - "notifications": { - "notification1": { - "notificationId": "notification1", - "startTime": "2011-01-01T00:00:00Z", - "endTime": "2099-01-01T00:00:00Z", - "createdAt": "2011-01-01T00:00:00Z", - "style": "DANGER", - "targetUser": "GENERAL", - "title": "E2E notif for general users", - "message": "

This notification is shown to general users

", - "shown": false - }, - "notification2": { - "notificationId": "notification2", - "startTime": "2011-02-02T00:00:00Z", - "endTime": "2099-02-02T00:00:00Z", - "createdAt": "2011-01-01T00:00:00Z", - "style": "SUCCESS", - "targetUser": "STUDENT", - "title": "E2E notif for students", - "message": "

This notification is shown to students only

", - "shown": false - }, - "notification3": { - "notificationId": "notification3", - "startTime": "2011-03-03T00:00:00Z", - "endTime": "2099-03-03T00:00:00Z", - "createdAt": "2011-01-01T00:00:00Z", - "style": "SUCCESS", - "targetUser": "INSTRUCTOR", - "title": "E2E notif for instructors", - "message": "

This notification is shown to instructors only

", - "shown": false - }, - "notification4": { - "notificationId": "notification4", - "startTime": "2011-04-04T00:00:00Z", - "endTime": "2099-04-04T00:00:00Z", - "createdAt": "2011-01-01T00:00:00Z", - "style": "WARNING", - "targetUser": "GENERAL", - "title": "E2E read notification", - "message": "

This notification has been read by the user

", - "shown": false - }, - "expiredNotification1": { - "notificationId": "expiredNotification1", - "startTime": "2011-01-01T00:00:00Z", - "endTime": "2011-02-02T00:00:00Z", - "createdAt": "2011-01-01T00:00:00Z", - "style": "DANGER", - "targetUser": "GENERAL", - "title": "E2E expired notification", - "message": "

This notification has expired

", - "shown": false - } } } diff --git a/src/e2e/resources/data/StudentNotificationsPageE2ETest_SqlEntities.json b/src/e2e/resources/data/StudentNotificationsPageE2ETest_SqlEntities.json new file mode 100644 index 00000000000..3ce6ca3d2cf --- /dev/null +++ b/src/e2e/resources/data/StudentNotificationsPageE2ETest_SqlEntities.json @@ -0,0 +1,73 @@ +{ + "accounts": { + "SNotifs.student": { + "id": "00000000-0000-4000-8000-000000000001", + "googleId": "tm.e2e.SNotifs.student", + "name": "Alice B", + "email": "SNotifs.student@gmail.tmt" + } + }, + "notifications": { + "notification1": { + "id": "00000000-0000-4000-8000-000000001101", + "startTime": "2011-01-01T00:00:00Z", + "endTime": "2099-01-01T00:00:00Z", + "style": "DANGER", + "targetUser": "GENERAL", + "title": "SNotifs.notification1", + "message": "

This notification is shown to general users

", + "shown": false + }, + "notification2": { + "id": "00000000-0000-4000-8000-000000001102", + "startTime": "2011-02-02T00:00:00Z", + "endTime": "2099-02-02T00:00:00Z", + "style": "SUCCESS", + "targetUser": "STUDENT", + "title": "SNotifs.notification2", + "message": "

This notification is shown to students only

", + "shown": false + }, + "notification3": { + "id": "00000000-0000-4000-8000-000000001103", + "startTime": "2011-03-03T00:00:00Z", + "endTime": "2099-03-03T00:00:00Z", + "style": "SUCCESS", + "targetUser": "INSTRUCTOR", + "title": "SNotifs.notification3", + "message": "

This notification is shown to instructors only

", + "shown": false + }, + "notification4": { + "id": "00000000-0000-4000-8000-000000001104", + "startTime": "2011-04-04T00:00:00Z", + "endTime": "2099-04-04T00:00:00Z", + "style": "WARNING", + "targetUser": "GENERAL", + "title": "SNotifs.notification4", + "message": "

This notification has been read by the user

", + "shown": false + }, + "expiredNotification1": { + "id": "00000000-0000-4000-8000-000000001105", + "startTime": "2011-01-01T00:00:00Z", + "endTime": "2011-02-02T00:00:00Z", + "style": "DANGER", + "targetUser": "GENERAL", + "title": "SNotifs.expiredNotification1", + "message": "

This notification has expired

", + "shown": false + } + }, + "readNotifications": { + "notification4SNotifs.student": { + "id": "00000000-0000-4000-8000-000000002100", + "account": { + "id": "00000000-0000-4000-8000-000000000001" + }, + "notification": { + "id": "00000000-0000-4000-8000-000000001104" + } + } + } +} diff --git a/src/it/java/teammates/it/sqllogic/core/DataBundleLogicIT.java b/src/it/java/teammates/it/sqllogic/core/DataBundleLogicIT.java index 28d2fb005a4..f8571037632 100644 --- a/src/it/java/teammates/it/sqllogic/core/DataBundleLogicIT.java +++ b/src/it/java/teammates/it/sqllogic/core/DataBundleLogicIT.java @@ -19,6 +19,7 @@ import teammates.common.datatransfer.questions.FeedbackTextQuestionDetails; import teammates.common.datatransfer.questions.FeedbackTextResponseDetails; import teammates.common.exception.EntityAlreadyExistsException; +import teammates.common.exception.EntityDoesNotExistException; import teammates.common.exception.InvalidParametersException; import teammates.it.test.BaseTestCaseWithSqlDatabaseAccess; import teammates.sqllogic.core.DataBundleLogic; @@ -238,7 +239,7 @@ public void testPersistDataBundle_typicalValues_persistedToDbCorrectly() throws @Test public void testRemoveDataBundle_typicalValues_removedCorrectly() - throws InvalidParametersException, EntityAlreadyExistsException { + throws InvalidParametersException, EntityAlreadyExistsException, EntityDoesNotExistException { SqlDataBundle dataBundle = loadSqlDataBundle("/DataBundleLogicIT.json"); dataBundleLogic.persistDataBundle(dataBundle); diff --git a/src/it/java/teammates/it/test/BaseTestCaseWithSqlDatabaseAccess.java b/src/it/java/teammates/it/test/BaseTestCaseWithSqlDatabaseAccess.java index 5d8d06fde2c..86eaceaf200 100644 --- a/src/it/java/teammates/it/test/BaseTestCaseWithSqlDatabaseAccess.java +++ b/src/it/java/teammates/it/test/BaseTestCaseWithSqlDatabaseAccess.java @@ -19,6 +19,7 @@ import teammates.common.datatransfer.SqlDataBundle; import teammates.common.exception.EntityAlreadyExistsException; +import teammates.common.exception.EntityDoesNotExistException; import teammates.common.exception.InvalidParametersException; import teammates.common.exception.SearchServiceException; import teammates.common.util.HibernateUtil; @@ -132,7 +133,7 @@ protected String getTestDataFolder() { * Persist data bundle into the db. */ protected void persistDataBundle(SqlDataBundle dataBundle) - throws InvalidParametersException, EntityAlreadyExistsException { + throws InvalidParametersException, EntityAlreadyExistsException, EntityDoesNotExistException { logic.persistDataBundle(dataBundle); } diff --git a/src/main/java/teammates/sqllogic/api/Logic.java b/src/main/java/teammates/sqllogic/api/Logic.java index 278009816d1..71286413c5a 100644 --- a/src/main/java/teammates/sqllogic/api/Logic.java +++ b/src/main/java/teammates/sqllogic/api/Logic.java @@ -1209,7 +1209,7 @@ public List getFeedbackQuestionsForInstructors( * Persists the given data bundle to the database. */ public SqlDataBundle persistDataBundle(SqlDataBundle dataBundle) - throws InvalidParametersException, EntityAlreadyExistsException { + throws InvalidParametersException, EntityAlreadyExistsException, EntityDoesNotExistException { return dataBundleLogic.persistDataBundle(dataBundle); } diff --git a/src/main/java/teammates/sqllogic/core/DataBundleLogic.java b/src/main/java/teammates/sqllogic/core/DataBundleLogic.java index deb23754fd7..6b7376a7e9b 100644 --- a/src/main/java/teammates/sqllogic/core/DataBundleLogic.java +++ b/src/main/java/teammates/sqllogic/core/DataBundleLogic.java @@ -7,6 +7,7 @@ import teammates.common.datatransfer.SqlDataBundle; import teammates.common.exception.EntityAlreadyExistsException; +import teammates.common.exception.EntityDoesNotExistException; import teammates.common.exception.InvalidParametersException; import teammates.common.exception.SearchServiceException; import teammates.common.util.JsonUtils; @@ -244,9 +245,11 @@ public static SqlDataBundle deserializeDataBundle(String jsonString) { * Persists data in the given {@link DataBundle} to the database. * * @throws InvalidParametersException if invalid data is encountered. + * @throws EntityDoesNotExistException if an entity was not found. + * (ReadNotification requires Account and Notification to be created) */ public SqlDataBundle persistDataBundle(SqlDataBundle dataBundle) - throws InvalidParametersException, EntityAlreadyExistsException { + throws InvalidParametersException, EntityAlreadyExistsException, EntityDoesNotExistException { if (dataBundle == null) { throw new InvalidParametersException("Null data bundle"); } @@ -264,6 +267,7 @@ public SqlDataBundle persistDataBundle(SqlDataBundle dataBundle) Collection responseComments = dataBundle.feedbackResponseComments.values(); Collection deadlineExtensions = dataBundle.deadlineExtensions.values(); Collection notifications = dataBundle.notifications.values(); + Collection readNotifications = dataBundle.readNotifications.values(); for (AccountRequest accountRequest : accountRequests) { accountRequestsLogic.createAccountRequest(accountRequest); @@ -314,6 +318,11 @@ public SqlDataBundle persistDataBundle(SqlDataBundle dataBundle) usersLogic.createStudent(student); } + for (ReadNotification readNotification : readNotifications) { + accountsLogic.updateReadNotifications(readNotification.getAccount().getGoogleId(), + readNotification.getNotification().getId(), readNotification.getNotification().getEndTime()); + } + for (DeadlineExtension deadlineExtension : deadlineExtensions) { deadlineExtensionsLogic.createDeadlineExtension(deadlineExtension); } diff --git a/src/main/java/teammates/ui/output/AccountData.java b/src/main/java/teammates/ui/output/AccountData.java index 433ccc8bad1..fc024a6ea99 100644 --- a/src/main/java/teammates/ui/output/AccountData.java +++ b/src/main/java/teammates/ui/output/AccountData.java @@ -36,7 +36,7 @@ public AccountData(Account account) { this.readNotifications = account.getReadNotifications() .stream() .collect(Collectors.toMap( - readNotification -> readNotification.getId().toString(), + readNotification -> readNotification.getNotification().getId().toString(), readNotification -> readNotification.getNotification().getEndTime().toEpochMilli())); } diff --git a/src/main/java/teammates/ui/webapi/CreateAccountAction.java b/src/main/java/teammates/ui/webapi/CreateAccountAction.java index 56f25ddaa72..63bde3a7f9d 100644 --- a/src/main/java/teammates/ui/webapi/CreateAccountAction.java +++ b/src/main/java/teammates/ui/webapi/CreateAccountAction.java @@ -71,7 +71,7 @@ public JsonResult execute() throws InvalidHttpRequestBodyException, InvalidOpera try { // persists sample data such as course, students, instructor and feedback sessions courseId = importAndPersistDemoData(instructorEmail, instructorName, instructorInstitution, timezone); - } catch (InvalidParametersException | EntityAlreadyExistsException e) { + } catch (InvalidParametersException | EntityAlreadyExistsException | EntityDoesNotExistException e) { // There should not be any invalid parameter here // EntityAlreadyExistsException should not be thrown as the generated demo course id should not exist. // If it is thrown, some programming error is the cause. @@ -138,7 +138,7 @@ private static String getDateString(Instant instant) { */ private String importAndPersistDemoData(String instructorEmail, String instructorName, String instructorInstitute, String timezone) - throws InvalidParametersException, EntityAlreadyExistsException { + throws InvalidParametersException, EntityAlreadyExistsException, EntityDoesNotExistException { String courseId = generateDemoCourseId(instructorEmail); Instant now = Instant.now(); diff --git a/src/main/java/teammates/ui/webapi/GetAccountAction.java b/src/main/java/teammates/ui/webapi/GetAccountAction.java index a44945edd36..8fd52fb5b9f 100644 --- a/src/main/java/teammates/ui/webapi/GetAccountAction.java +++ b/src/main/java/teammates/ui/webapi/GetAccountAction.java @@ -1,6 +1,5 @@ package teammates.ui.webapi; -import teammates.common.datatransfer.attributes.AccountAttributes; import teammates.common.util.Const; import teammates.storage.sqlentity.Account; import teammates.ui.output.AccountData; @@ -14,21 +13,14 @@ class GetAccountAction extends AdminOnlyAction { public JsonResult execute() { String googleId = getNonNullRequestParamValue(Const.ParamsNames.INSTRUCTOR_ID); - AccountAttributes accountInfo = logic.getAccount(googleId); + Account account = sqlLogic.getAccountForGoogleId(googleId); - if (accountInfo == null || accountInfo.isMigrated()) { - Account account = sqlLogic.getAccountForGoogleId(googleId); - - if (account == null) { - throw new EntityNotFoundException("Account does not exist."); - } - - AccountData output = new AccountData(account); - return new JsonResult(output); - } else { - AccountData output = new AccountData(accountInfo); - return new JsonResult(output); + if (account == null) { + throw new EntityNotFoundException("Account does not exist."); } + + AccountData output = new AccountData(account); + return new JsonResult(output); } } diff --git a/src/main/java/teammates/ui/webapi/MarkNotificationAsReadAction.java b/src/main/java/teammates/ui/webapi/MarkNotificationAsReadAction.java index bd46b3273a8..31cbe29816e 100644 --- a/src/main/java/teammates/ui/webapi/MarkNotificationAsReadAction.java +++ b/src/main/java/teammates/ui/webapi/MarkNotificationAsReadAction.java @@ -34,12 +34,6 @@ public ActionResult execute() throws InvalidHttpRequestBodyException, InvalidOpe Instant endTime = Instant.ofEpochMilli(readNotificationCreateRequest.getEndTimestamp()); try { - if (!isAccountMigrated(userInfo.getId())) { - List readNotifications = - logic.updateReadNotifications(userInfo.getId(), notificationId.toString(), endTime); - ReadNotificationsData output = new ReadNotificationsData(readNotifications); - return new JsonResult(output); - } List readNotifications = sqlLogic.updateReadNotifications(userInfo.getId(), notificationId, endTime); ReadNotificationsData output = new ReadNotificationsData( diff --git a/src/main/java/teammates/ui/webapi/PutSqlDataBundleAction.java b/src/main/java/teammates/ui/webapi/PutSqlDataBundleAction.java index 998617de095..4b4a01fa4a3 100644 --- a/src/main/java/teammates/ui/webapi/PutSqlDataBundleAction.java +++ b/src/main/java/teammates/ui/webapi/PutSqlDataBundleAction.java @@ -2,6 +2,7 @@ import teammates.common.datatransfer.SqlDataBundle; import teammates.common.exception.EntityAlreadyExistsException; +import teammates.common.exception.EntityDoesNotExistException; import teammates.common.exception.InvalidParametersException; import teammates.common.util.Config; import teammates.common.util.JsonUtils; @@ -34,6 +35,8 @@ public JsonResult execute() throws InvalidHttpRequestBodyException, InvalidOpera throw new InvalidHttpRequestBodyException(e); } catch (EntityAlreadyExistsException e) { throw new InvalidOperationException("Some entities in the databundle already exist", e); + } catch (EntityDoesNotExistException e) { + throw new EntityNotFoundException(e); } return new JsonResult(JsonUtils.toJson(dataBundle)); diff --git a/src/test/java/teammates/architecture/ArchitectureTest.java b/src/test/java/teammates/architecture/ArchitectureTest.java index 8ea1d51aa86..1113db61372 100644 --- a/src/test/java/teammates/architecture/ArchitectureTest.java +++ b/src/test/java/teammates/architecture/ArchitectureTest.java @@ -351,7 +351,13 @@ public boolean apply(JavaClass input) { } }) .orShould().accessClassesThat().resideInAPackage(includeSubpackages(LOGIC_PACKAGE)) - .orShould().accessClassesThat().resideInAPackage(includeSubpackages(UI_PACKAGE)) + .orShould().accessClassesThat(new DescribedPredicate<>("") { + @Override + public boolean apply(JavaClass input) { + return input.getPackageName().startsWith(UI_PACKAGE) + && !input.getPackageName().startsWith(UI_OUTPUT_PACKAGE); + } + }) .check(forClasses(E2E_PACKAGE)); noClasses().that().resideInAPackage(includeSubpackages(E2E_PACKAGE)) diff --git a/src/test/java/teammates/test/AbstractBackDoor.java b/src/test/java/teammates/test/AbstractBackDoor.java index c5c46eddfa2..5cbcc012808 100644 --- a/src/test/java/teammates/test/AbstractBackDoor.java +++ b/src/test/java/teammates/test/AbstractBackDoor.java @@ -12,6 +12,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.UUID; import java.util.stream.Collectors; import org.apache.http.HttpEntity; @@ -29,6 +30,9 @@ import org.apache.http.impl.client.HttpClients; import org.apache.http.message.BasicNameValuePair; +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; + import teammates.common.datatransfer.DataBundle; import teammates.common.datatransfer.FeedbackParticipantType; import teammates.common.datatransfer.SqlDataBundle; @@ -268,7 +272,7 @@ public String removeAndRestoreDataBundle(DataBundle dataBundle) throws HttpReque /** * Removes and restores given data in the database. This method is to be called on test startup. */ - public String removeAndRestoreSqlDataBundle(SqlDataBundle dataBundle) throws HttpRequestFailedException { + public SqlDataBundle removeAndRestoreSqlDataBundle(SqlDataBundle dataBundle) throws HttpRequestFailedException { removeSqlDataBundle(dataBundle); ResponseBodyAndCode putRequestOutput = executePostRequest(Const.ResourceURIs.SQL_DATABUNDLE, null, JsonUtils.toJson(dataBundle)); @@ -276,7 +280,11 @@ public String removeAndRestoreSqlDataBundle(SqlDataBundle dataBundle) throws Htt throw new HttpRequestFailedException("Request failed: [" + putRequestOutput.responseCode + "] " + putRequestOutput.responseBody); } - return putRequestOutput.responseBody; + + JsonObject jsonObject = JsonParser.parseString(putRequestOutput.responseBody).getAsJsonObject(); + // data bundle is nested under message key + String message = jsonObject.get("message").getAsString(); + return JsonUtils.fromJson(message, SqlDataBundle.class); } /** @@ -342,9 +350,9 @@ public String putSqlDocuments(SqlDataBundle dataBundle) throws HttpRequestFailed } /** - * Gets an account from the database. + * Gets account data from the database. */ - public AccountAttributes getAccount(String googleId) { + public AccountData getAccountData(String googleId) { Map params = new HashMap<>(); params.put(Const.ParamsNames.INSTRUCTOR_ID, googleId); ResponseBodyAndCode response = executeGetRequest(Const.ResourceURIs.ACCOUNT, params); @@ -352,7 +360,14 @@ public AccountAttributes getAccount(String googleId) { return null; } - AccountData accountData = JsonUtils.fromJson(response.responseBody, AccountData.class); + return JsonUtils.fromJson(response.responseBody, AccountData.class); + } + + /** + * Gets an account from the database. + */ + public AccountAttributes getAccount(String googleId) { + AccountData accountData = getAccountData(googleId); return AccountAttributes.builder(accountData.getGoogleId()) .withName(accountData.getName()) .withEmail(accountData.getEmail()) @@ -886,6 +901,15 @@ public void deleteNotification(String notificationId) { executeDeleteRequest(Const.ResourceURIs.NOTIFICATION, params); } + /** + * Deletes a notification from the database. + */ + public void deleteNotification(UUID notificationId) { + Map params = new HashMap<>(); + params.put(Const.ParamsNames.NOTIFICATION_ID, notificationId.toString()); + executeDeleteRequest(Const.ResourceURIs.NOTIFICATION, params); + } + /** * Gets a deadline extension from the database. */ diff --git a/src/test/java/teammates/test/BaseTestCaseWithDatabaseAccess.java b/src/test/java/teammates/test/BaseTestCaseWithDatabaseAccess.java index b023e68e087..d77c2639a6c 100644 --- a/src/test/java/teammates/test/BaseTestCaseWithDatabaseAccess.java +++ b/src/test/java/teammates/test/BaseTestCaseWithDatabaseAccess.java @@ -1,6 +1,7 @@ package teammates.test; import teammates.common.datatransfer.DataBundle; +import teammates.common.datatransfer.SqlDataBundle; import teammates.common.datatransfer.attributes.AccountAttributes; import teammates.common.datatransfer.attributes.AccountRequestAttributes; import teammates.common.datatransfer.attributes.CourseAttributes; @@ -268,6 +269,21 @@ protected void removeAndRestoreDataBundle(DataBundle testData) { protected abstract boolean doRemoveAndRestoreDataBundle(DataBundle testData); + protected SqlDataBundle removeAndRestoreSqlDataBundle(SqlDataBundle testData) { + int retryLimit = OPERATION_RETRY_COUNT; + SqlDataBundle dataBundle = doRemoveAndRestoreSqlDataBundle(testData); + while (dataBundle == null && retryLimit > 0) { + retryLimit--; + print("Re-trying removeAndRestoreDataBundle"); + ThreadHelper.waitFor(OPERATION_RETRY_DELAY_IN_MS); + dataBundle = doRemoveAndRestoreSqlDataBundle(testData); + } + assertNotNull(dataBundle); + return dataBundle; + } + + protected abstract SqlDataBundle doRemoveAndRestoreSqlDataBundle(SqlDataBundle testData); + protected void putDocuments(DataBundle testData) { int retryLimit = OPERATION_RETRY_COUNT; boolean isOperationSuccess = doPutDocuments(testData); diff --git a/src/test/java/teammates/test/BaseTestCaseWithLocalDatabaseAccess.java b/src/test/java/teammates/test/BaseTestCaseWithLocalDatabaseAccess.java index ad046e05642..2370c8c3991 100644 --- a/src/test/java/teammates/test/BaseTestCaseWithLocalDatabaseAccess.java +++ b/src/test/java/teammates/test/BaseTestCaseWithLocalDatabaseAccess.java @@ -16,6 +16,7 @@ import com.googlecode.objectify.util.Closeable; import teammates.common.datatransfer.DataBundle; +import teammates.common.datatransfer.SqlDataBundle; import teammates.common.datatransfer.attributes.AccountAttributes; import teammates.common.datatransfer.attributes.AccountRequestAttributes; import teammates.common.datatransfer.attributes.CourseAttributes; @@ -30,6 +31,7 @@ import teammates.common.util.HibernateUtil; import teammates.logic.api.LogicExtension; import teammates.logic.core.LogicStarter; +import teammates.sqllogic.api.Logic; import teammates.storage.api.OfyHelper; import teammates.storage.search.AccountRequestSearchManager; import teammates.storage.search.InstructorSearchManager; @@ -52,6 +54,7 @@ public abstract class BaseTestCaseWithLocalDatabaseAccess extends BaseTestCaseWi .setStoreOnDisk(false) .build(); private final LogicExtension logic = new LogicExtension(); + private Logic sqlLogic; private Closeable closeable; @BeforeSuite @@ -59,6 +62,7 @@ public void setupDbLayer() throws Exception { PGSQL.start(); HibernateUtil.buildSessionFactory(PGSQL.getJdbcUrl(), PGSQL.getUsername(), PGSQL.getPassword()); teammates.sqllogic.core.LogicStarter.initializeDependencies(); + sqlLogic = Logic.inst(); LOCAL_DATASTORE_HELPER.start(); DatastoreOptions options = LOCAL_DATASTORE_HELPER.getOptions(); @@ -188,6 +192,17 @@ protected boolean doRemoveAndRestoreDataBundle(DataBundle dataBundle) { } } + @Override + protected SqlDataBundle doRemoveAndRestoreSqlDataBundle(SqlDataBundle dataBundle) { + try { + sqlLogic.removeDataBundle(dataBundle); + return sqlLogic.persistDataBundle(dataBundle); + } catch (Exception e) { + e.printStackTrace(); + return null; + } + } + @Override protected boolean doPutDocuments(DataBundle dataBundle) { try {