From 349a91bc498d09206390c3fcd5c6561b04bb3b3e Mon Sep 17 00:00:00 2001 From: Zhang Ziqing Date: Sun, 13 Mar 2022 23:07:07 +0800 Subject: [PATCH 1/9] Add test for NotificationAttributes --- .../attributes/NotificationAttributes.java | 2 + .../NotificationAttributesTest.java | 270 ++++++++++++++++++ 2 files changed, 272 insertions(+) create mode 100644 src/test/java/teammates/common/datatransfer/attributes/NotificationAttributesTest.java diff --git a/src/main/java/teammates/common/datatransfer/attributes/NotificationAttributes.java b/src/main/java/teammates/common/datatransfer/attributes/NotificationAttributes.java index e4a8971c3a6..87d7dc60e04 100644 --- a/src/main/java/teammates/common/datatransfer/attributes/NotificationAttributes.java +++ b/src/main/java/teammates/common/datatransfer/attributes/NotificationAttributes.java @@ -365,6 +365,8 @@ public B withEndTime(Instant endTime) { } public B withType(NotificationType type) { + assert type != null; + updateOptions.typeOption = UpdateOption.of(type); return thisBuilder; } diff --git a/src/test/java/teammates/common/datatransfer/attributes/NotificationAttributesTest.java b/src/test/java/teammates/common/datatransfer/attributes/NotificationAttributesTest.java new file mode 100644 index 00000000000..b7b27364f94 --- /dev/null +++ b/src/test/java/teammates/common/datatransfer/attributes/NotificationAttributesTest.java @@ -0,0 +1,270 @@ +package teammates.common.datatransfer.attributes; + +import java.time.Instant; + +import org.testng.annotations.Test; + +import teammates.common.datatransfer.NotificationTargetUser; +import teammates.common.datatransfer.NotificationType; +import teammates.storage.entity.Notification; +import teammates.test.BaseTestCase; + +/** + * SUT: {@link NotificationAttributes}. + */ +public class NotificationAttributesTest extends BaseTestCase { + private static final Instant startTime1 = Instant.now().plusSeconds(3600); + private static final Instant endTime1 = Instant.now().plusSeconds(7200); + private static final Instant startTime2 = Instant.now().plusSeconds(1000); + private static final Instant endTime2 = Instant.now().plusSeconds(10000); + + @Test + public void testValueOf_withAllFieldPopulatedNotificationAttributes_shouldGenerateAttributesCorrectly() { + Notification notification = new Notification("valid-notification-id", + startTime1, endTime1, + NotificationType.DEPRECATION, NotificationTargetUser.INSTRUCTOR, + "valid notification title", "valid notification message", false, + Instant.now(), Instant.now()); + NotificationAttributes nfa = NotificationAttributes.valueOf(notification); + assertEquals(notification.getNotificationId(), nfa.getNotificationId()); + assertEquals(notification.getStartTime(), nfa.getStartTime()); + assertEquals(notification.getEndTime(), nfa.getEndTime()); + assertEquals(notification.getType(), nfa.getType()); + assertEquals(notification.getTargetUser(), nfa.getTargetUser()); + assertEquals(notification.getTitle(), nfa.getTitle()); + assertEquals(notification.getMessage(), nfa.getMessage()); + assertEquals(notification.isShown(), nfa.isShown()); + assertEquals(notification.getCreatedAt(), nfa.getCreatedAt()); + assertEquals(notification.getUpdatedAt(), nfa.getUpdatedAt()); + } + + @Test + public void testBuilder_withNullArguments_shouldThrowException() { + assertThrows(AssertionError.class, () -> { + NotificationAttributes + .builder(null) + .build(); + }); + + assertThrows(AssertionError.class, () -> { + NotificationAttributes + .builder("notificationId") + .withStartTime(null) + .build(); + }); + + assertThrows(AssertionError.class, () -> { + NotificationAttributes + .builder("notificationId") + .withEndTime(null) + .build(); + }); + + assertThrows(AssertionError.class, () -> { + NotificationAttributes + .builder("notificationId") + .withType(null) + .build(); + }); + + assertThrows(AssertionError.class, () -> { + NotificationAttributes + .builder("notificationId") + .withTargetUser(null) + .build(); + }); + + assertThrows(AssertionError.class, () -> { + NotificationAttributes + .builder("notificationId") + .withTitle(null) + .build(); + }); + + assertThrows(AssertionError.class, () -> { + NotificationAttributes + .builder("notificationId") + .withMessage(null) + .build(); + }); + } + + @Test + public void testBuilder_withTypicalData_shouldBuildCorrectAttributes() { + Notification notification = new Notification(startTime1, endTime1, + NotificationType.TIPS, NotificationTargetUser.GENERAL, + "Another tip for usage", "Here the message starts"); + NotificationAttributes nfa = NotificationAttributes.valueOf(notification); + assertEquals(startTime1, nfa.getStartTime()); + assertEquals(endTime1, nfa.getEndTime()); + assertEquals(NotificationType.TIPS, nfa.getType()); + assertEquals(NotificationTargetUser.GENERAL, nfa.getTargetUser()); + assertEquals("Another tip for usage", nfa.getTitle()); + assertEquals("Here the message starts", nfa.getMessage()); + } + + @Test + public void testCopyConstructor_shouldDoDeepCopyOfNotificationDetails() { + NotificationAttributes nfa1 = NotificationAttributes.builder("notificationId") + .withStartTime(startTime1) + .withEndTime(endTime1) + .withType(NotificationType.DEPRECATION) + .withTargetUser(NotificationTargetUser.INSTRUCTOR) + .withTitle("valid notification title") + .withMessage("The first message") + .build(); + NotificationAttributes nfa2 = nfa1.getCopy(); + nfa2.setMessage("The second message"); + + assertEquals(nfa1.getMessage(), "The first message"); + assertEquals(nfa2.getMessage(), "The second message"); + } + + @Test + public void testUpdateOptions_withTypicalUpdateOptions_shouldUpdateAttributeCorrectly() { + NotificationAttributes.UpdateOptions updateOptions = + NotificationAttributes.updateOptionsBuilder("notificationId") + .withStartTime(startTime2) + .withEndTime(endTime2) + .withType(NotificationType.VERSION_NOTE) + .withTargetUser(NotificationTargetUser.STUDENT) + .withTitle("The edited title") + .withMessage("The edited message") + .build(); + + assertEquals("notificationId", updateOptions.getNotificationId()); + + NotificationAttributes notificationAttributes = + NotificationAttributes.builder("notificationId") + .withStartTime(startTime1) + .withEndTime(endTime1) + .withType(NotificationType.DEPRECATION) + .withTargetUser(NotificationTargetUser.INSTRUCTOR) + .withTitle("valid notification title") + .withMessage("The first message") + .build(); + + notificationAttributes.update(updateOptions); + + assertEquals(startTime2, notificationAttributes.getStartTime()); + assertEquals(endTime2, notificationAttributes.getEndTime()); + assertEquals(NotificationType.VERSION_NOTE, notificationAttributes.getType()); + assertEquals(NotificationTargetUser.STUDENT, notificationAttributes.getTargetUser()); + assertEquals("The edited title", notificationAttributes.getTitle()); + assertEquals("The edited message", notificationAttributes.getMessage()); + } + + @Test + public void testUpdateOptionsBuilder_withNullInput_shouldFailWithAssertionError() { + assertThrows(AssertionError.class, () -> + NotificationAttributes.updateOptionsBuilder((String) null)); + assertThrows(AssertionError.class, () -> + NotificationAttributes.updateOptionsBuilder("notificationId") + .withStartTime(null)); + assertThrows(AssertionError.class, () -> + NotificationAttributes.updateOptionsBuilder("notificationId") + .withEndTime(null)); + assertThrows(AssertionError.class, () -> + NotificationAttributes.updateOptionsBuilder("notificationId") + .withType(null)); + assertThrows(AssertionError.class, () -> + NotificationAttributes.updateOptionsBuilder("notificationId") + .withTargetUser(null)); + assertThrows(AssertionError.class, () -> + NotificationAttributes.updateOptionsBuilder("notificationId") + .withTitle(null)); + assertThrows(AssertionError.class, () -> + NotificationAttributes.updateOptionsBuilder("notificationId") + .withMessage(null)); + } + + @Test + public void testEquals() { + NotificationAttributes notificationAttributes = + NotificationAttributes.builder("notificationId") + .withStartTime(startTime1) + .withEndTime(endTime1) + .withType(NotificationType.DEPRECATION) + .withTargetUser(NotificationTargetUser.INSTRUCTOR) + .withTitle("valid notification title") + .withMessage("valid message") + .build(); + + // When the two notifications are exact copies + NotificationAttributes notificationAttributesCopy = notificationAttributes.getCopy(); + + assertTrue(notificationAttributes.equals(notificationAttributesCopy)); + + // When the two notifications have same values but created at different time + NotificationAttributes notificationAttributesSimilar = + NotificationAttributes.builder("notificationId") + .withStartTime(startTime1) + .withEndTime(endTime1) + .withType(NotificationType.DEPRECATION) + .withTargetUser(NotificationTargetUser.INSTRUCTOR) + .withTitle("valid notification title") + .withMessage("valid message") + .build(); + + assertTrue(notificationAttributes.equals(notificationAttributesSimilar)); + + NotificationAttributes notificationAttributesDifferent = + NotificationAttributes.builder("differentId") + .withStartTime(startTime1) + .withEndTime(endTime1) + .withType(NotificationType.DEPRECATION) + .withTargetUser(NotificationTargetUser.INSTRUCTOR) + .withTitle("valid notification title") + .withMessage("valid message") + .build(); + + assertFalse(notificationAttributes.equals(notificationAttributesDifferent)); + + // When the other object is of different class + assertFalse(notificationAttributes.equals(3)); + } + + @Test + public void testHashCode() { + NotificationAttributes notificationAttributes = + NotificationAttributes.builder("notificationId") + .withStartTime(startTime1) + .withEndTime(endTime1) + .withType(NotificationType.DEPRECATION) + .withTargetUser(NotificationTargetUser.INSTRUCTOR) + .withTitle("valid notification title") + .withMessage("valid message") + .build(); + + // When the two notifications are exact copies + NotificationAttributes notificationAttributesCopy = notificationAttributes.getCopy(); + + assertTrue(notificationAttributes.hashCode() == notificationAttributesCopy.hashCode()); + + // When the two notifications have same values but created at different time + NotificationAttributes notificationAttributesSimilar = + NotificationAttributes.builder("notificationId") + .withStartTime(startTime1) + .withEndTime(endTime1) + .withType(NotificationType.DEPRECATION) + .withTargetUser(NotificationTargetUser.INSTRUCTOR) + .withTitle("valid notification title") + .withMessage("valid message") + .build(); + + assertTrue(notificationAttributes.hashCode() == notificationAttributesSimilar.hashCode()); + + NotificationAttributes notificationAttributesDifferent = + NotificationAttributes.builder("differentId") + .withStartTime(startTime1) + .withEndTime(endTime1) + .withType(NotificationType.DEPRECATION) + .withTargetUser(NotificationTargetUser.INSTRUCTOR) + .withTitle("valid notification title") + .withMessage("valid message") + .build(); + + assertFalse(notificationAttributes.hashCode() == notificationAttributesDifferent.hashCode()); + } + +} From d146b6904611cc4a2a812ec05cb1ebc1b8d01677 Mon Sep 17 00:00:00 2001 From: Zhang Ziqing Date: Mon, 14 Mar 2022 01:02:07 +0800 Subject: [PATCH 2/9] Add notifications to typicalDataBundles and init GET api test --- .../common/datatransfer/DataBundle.java | 2 + .../teammates/logic/core/DataBundleLogic.java | 9 +++ .../ui/webapi/GetNotificationAction.java | 4 + .../ui/webapi/GetNotificationActionTest.java | 80 +++++++++++++++++++ .../resources/data/typicalDataBundle.json | 38 +++++++++ 5 files changed, 133 insertions(+) create mode 100644 src/test/java/teammates/ui/webapi/GetNotificationActionTest.java diff --git a/src/main/java/teammates/common/datatransfer/DataBundle.java b/src/main/java/teammates/common/datatransfer/DataBundle.java index 2286f55592b..a7919d68147 100644 --- a/src/main/java/teammates/common/datatransfer/DataBundle.java +++ b/src/main/java/teammates/common/datatransfer/DataBundle.java @@ -11,6 +11,7 @@ import teammates.common.datatransfer.attributes.FeedbackResponseCommentAttributes; import teammates.common.datatransfer.attributes.FeedbackSessionAttributes; import teammates.common.datatransfer.attributes.InstructorAttributes; +import teammates.common.datatransfer.attributes.NotificationAttributes; import teammates.common.datatransfer.attributes.StudentAttributes; import teammates.common.datatransfer.attributes.StudentProfileAttributes; @@ -31,4 +32,5 @@ public class DataBundle { public Map feedbackResponses = new LinkedHashMap<>(); public Map feedbackResponseComments = new LinkedHashMap<>(); public Map profiles = new LinkedHashMap<>(); + public Map notifications = new LinkedHashMap<>(); } diff --git a/src/main/java/teammates/logic/core/DataBundleLogic.java b/src/main/java/teammates/logic/core/DataBundleLogic.java index c5973025b33..02e3aae3ed0 100644 --- a/src/main/java/teammates/logic/core/DataBundleLogic.java +++ b/src/main/java/teammates/logic/core/DataBundleLogic.java @@ -19,6 +19,7 @@ import teammates.common.datatransfer.attributes.FeedbackResponseCommentAttributes; import teammates.common.datatransfer.attributes.FeedbackSessionAttributes; import teammates.common.datatransfer.attributes.InstructorAttributes; +import teammates.common.datatransfer.attributes.NotificationAttributes; import teammates.common.datatransfer.attributes.StudentAttributes; import teammates.common.datatransfer.attributes.StudentProfileAttributes; import teammates.common.exception.InvalidParametersException; @@ -33,6 +34,7 @@ import teammates.storage.api.FeedbackResponsesDb; import teammates.storage.api.FeedbackSessionsDb; import teammates.storage.api.InstructorsDb; +import teammates.storage.api.NotificationsDb; import teammates.storage.api.ProfilesDb; import teammates.storage.api.StudentsDb; @@ -55,6 +57,7 @@ public final class DataBundleLogic { private final FeedbackQuestionsDb fqDb = FeedbackQuestionsDb.inst(); private final FeedbackResponsesDb frDb = FeedbackResponsesDb.inst(); private final FeedbackResponseCommentsDb fcDb = FeedbackResponseCommentsDb.inst(); + private final NotificationsDb nfDb = NotificationsDb.inst(); private DataBundleLogic() { // prevent initialization @@ -90,6 +93,7 @@ public DataBundle persistDataBundle(DataBundle dataBundle) throws InvalidParamet Collection questions = dataBundle.feedbackQuestions.values(); Collection responses = dataBundle.feedbackResponses.values(); Collection responseComments = dataBundle.feedbackResponseComments.values(); + Collection notifications = dataBundle.notifications.values(); // For ensuring only one account per Google ID is created Map googleIdAccountMap = new HashMap<>(); @@ -115,6 +119,7 @@ public DataBundle persistDataBundle(DataBundle dataBundle) throws InvalidParamet List newFeedbackResponses = frDb.putEntities(responses); List newFeedbackResponseComments = fcDb.putEntities(responseComments); + List newNotifications = nfDb.putEntities(notifications); updateDataBundleValue(newAccounts, dataBundle.accounts); updateDataBundleValue(newAccountRequests, dataBundle.accountRequests); @@ -126,6 +131,7 @@ public DataBundle persistDataBundle(DataBundle dataBundle) throws InvalidParamet updateDataBundleValue(createdQuestions, dataBundle.feedbackQuestions); updateDataBundleValue(newFeedbackResponses, dataBundle.feedbackResponses); updateDataBundleValue(newFeedbackResponseComments, dataBundle.feedbackResponseComments); + updateDataBundleValue(newNotifications, dataBundle.notifications); return dataBundle; @@ -379,6 +385,9 @@ public void removeDataBundle(DataBundle dataBundle) { dataBundle.accountRequests.values().forEach(accountRequest -> { accountRequestsDb.deleteAccountRequest(accountRequest.getEmail(), accountRequest.getInstitute()); }); + dataBundle.notifications.values().forEach(notification -> { + nfDb.deleteNotification(notification.getNotificationId()); + }); } private void deleteCourses(Collection courses) { diff --git a/src/main/java/teammates/ui/webapi/GetNotificationAction.java b/src/main/java/teammates/ui/webapi/GetNotificationAction.java index a4dc9263d50..1640636ff02 100644 --- a/src/main/java/teammates/ui/webapi/GetNotificationAction.java +++ b/src/main/java/teammates/ui/webapi/GetNotificationAction.java @@ -46,6 +46,10 @@ public JsonResult execute() { if (targetUser == NotificationTargetUser.GENERAL) { throw new InvalidHttpParameterException(INVALID_TARGET_USER); } + if (targetUser == NotificationTargetUser.INSTRUCTOR && userInfo.isStudent + || targetUser == NotificationTargetUser.STUDENT && userInfo.isInstructor) { + throw new InvalidHttpParameterException(INVALID_TARGET_USER); + } notificationAttributes = logic.getActiveNotificationsByTargetUser(targetUser); } diff --git a/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java b/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java new file mode 100644 index 00000000000..4889e1b71b8 --- /dev/null +++ b/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java @@ -0,0 +1,80 @@ +package teammates.ui.webapi; + +import java.util.List; + +import org.testng.annotations.Test; + +import teammates.common.datatransfer.NotificationTargetUser; +import teammates.common.datatransfer.NotificationType; +import teammates.common.datatransfer.attributes.AccountAttributes; +import teammates.common.datatransfer.attributes.FeedbackSessionAttributes; +import teammates.common.datatransfer.attributes.InstructorAttributes; +import teammates.common.datatransfer.attributes.NotificationAttributes; +import teammates.common.datatransfer.attributes.StudentProfileAttributes; +import teammates.common.util.Const; +import teammates.ui.output.NotificationData; +import teammates.ui.output.NotificationsData; +import teammates.ui.output.StudentProfileData; +import teammates.ui.request.Intent; + +/** + * SUT: {@link GetNotificationAction}. + */ +public class GetNotificationActionTest extends BaseActionTest { + + private NotificationAttributes notificationAttributes; + + @Override + String getActionUri() { + return Const.ResourceURIs.NOTIFICATION; + } + + @Override + String getRequestMethod() { + return GET; + } + + @Override + protected void prepareTestData() { + removeAndRestoreTypicalDataBundle(); + } + + @Test + @Override + protected void testExecute() { + // See independent test cases + } + + @Override + protected void testAccessControl(){ + verifyAnyUserCanAccess(); + } + + @Test + public void testExecute_withFullDetailIntentForNonAdmin_shouldReturnDataWithFullDetail() { + InstructorAttributes instructor = typicalBundle.instructors.get("instructor1OfCourse1"); + loginAsInstructor(instructor.getGoogleId()); + NotificationAttributes notification = typicalBundle.notifications.get("notification-1"); + + String[] submissionParams = new String[] { + Const.ParamsNames.NOTIFICATION_TARGET_USER, NotificationTargetUser.INSTRUCTOR.toString(), + Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true), + }; + + GetNotificationAction action = getAction(submissionParams); + JsonResult jsonResult = getJsonResult(action); + + NotificationsData output = (NotificationsData) jsonResult.getOutput(); + List notifications = output.getNotifications(); + + assertEquals(2, logic.getActiveNotificationsByTargetUser(NotificationTargetUser.INSTRUCTOR).size()); + assertEquals(2, notifications.size()); + + NotificationData firstNotification = notifications.get(0); + assertEquals("notification-3", firstNotification.getNotificationId()); + assertEquals(NotificationType.VERSION_NOTE, firstNotification.getNotificationType()); + assertEquals(NotificationTargetUser.INSTRUCTOR, firstNotification.getTargetUser()); + assertEquals("The first version note", firstNotification.getTitle()); + assertEquals("The version note content", firstNotification.getMessage()); + } +} diff --git a/src/test/resources/data/typicalDataBundle.json b/src/test/resources/data/typicalDataBundle.json index dc69804fc02..8fe3a512849 100644 --- a/src/test/resources/data/typicalDataBundle.json +++ b/src/test/resources/data/typicalDataBundle.json @@ -1869,5 +1869,43 @@ "institute": "TEAMMATES Test Institute 2", "createdAt": "2011-01-01T00:00:00Z" } + }, + "notifications": { + "notification-1": { + "notificationId": "notification-1", + "startTime": "2011-01-01T00:00:00Z", + "endTime": "2024-01-01T00:00:00Z", + "createdAt": "2011-01-01T00:00:00Z", + "updatedAt": "2011-01-01T00:00:00Z", + "type": "DEPRECATION", + "targetUser": "GENERAL", + "title": "A deprecation note", + "message": "Deprecation happens in three minutes", + "shown": false + }, + "notification-2": { + "notificationId": "notification-2", + "startTime": "2011-02-02T00:00:00Z", + "endTime": "2024-02-02T00:00:00Z", + "createdAt": "2011-01-01T00:00:00Z", + "updatedAt": "2011-01-01T00:00:00Z", + "type": "VERSION_NOTE", + "targetUser": "STUDENT", + "title": "A note for update", + "message": "Exciting features", + "shown": false + }, + "notification-3": { + "notificationId": "notification-3", + "startTime": "2011-03-03T00:00:00Z", + "endTime": "2024-03-03T00:00:00Z", + "createdAt": "2011-01-01T00:00:00Z", + "updatedAt": "2011-01-01T00:00:00Z", + "type": "VERSION_NOTE", + "targetUser": "INSTRUCTOR", + "title": "The first version note", + "message": "The version note content", + "shown": false + } } } From 7270e4cb59693f00148308dc1093749f366f86ca Mon Sep 17 00:00:00 2001 From: Zhang Ziqing Date: Mon, 14 Mar 2022 02:15:47 +0800 Subject: [PATCH 3/9] Add tests for GET api --- .../attributes/NotificationAttributes.java | 2 +- .../NotificationAttributesTest.java | 56 ++++++------ .../ui/webapi/GetNotificationActionTest.java | 91 ++++++++++++++----- .../resources/data/typicalDataBundle.json | 36 ++++++++ 4 files changed, 134 insertions(+), 51 deletions(-) diff --git a/src/main/java/teammates/common/datatransfer/attributes/NotificationAttributes.java b/src/main/java/teammates/common/datatransfer/attributes/NotificationAttributes.java index 87d7dc60e04..5d405ff10e9 100644 --- a/src/main/java/teammates/common/datatransfer/attributes/NotificationAttributes.java +++ b/src/main/java/teammates/common/datatransfer/attributes/NotificationAttributes.java @@ -160,7 +160,7 @@ public void setUpdatedAt(Instant updatedAt) { } /** - * Sorts the list of notifications by the start time. + * Sorts the list of notifications by the start time, with the latest as the first. */ public static void sortByStartTime(List notifications) { notifications.sort(Comparator.comparing(NotificationAttributes::getStartTime)); diff --git a/src/test/java/teammates/common/datatransfer/attributes/NotificationAttributesTest.java b/src/test/java/teammates/common/datatransfer/attributes/NotificationAttributesTest.java index b7b27364f94..e7b918d702b 100644 --- a/src/test/java/teammates/common/datatransfer/attributes/NotificationAttributesTest.java +++ b/src/test/java/teammates/common/datatransfer/attributes/NotificationAttributesTest.java @@ -13,15 +13,15 @@ * SUT: {@link NotificationAttributes}. */ public class NotificationAttributesTest extends BaseTestCase { - private static final Instant startTime1 = Instant.now().plusSeconds(3600); - private static final Instant endTime1 = Instant.now().plusSeconds(7200); - private static final Instant startTime2 = Instant.now().plusSeconds(1000); - private static final Instant endTime2 = Instant.now().plusSeconds(10000); + private static final Instant STARTTIME_ONE = Instant.now().plusSeconds(3600); + private static final Instant ENDTIME_ONE = Instant.now().plusSeconds(7200); + private static final Instant STARTTIME_TWO = Instant.now().plusSeconds(1000); + private static final Instant ENDTIME_TWO = Instant.now().plusSeconds(10000); @Test public void testValueOf_withAllFieldPopulatedNotificationAttributes_shouldGenerateAttributesCorrectly() { Notification notification = new Notification("valid-notification-id", - startTime1, endTime1, + STARTTIME_ONE, ENDTIME_ONE, NotificationType.DEPRECATION, NotificationTargetUser.INSTRUCTOR, "valid notification title", "valid notification message", false, Instant.now(), Instant.now()); @@ -91,12 +91,12 @@ public void testBuilder_withNullArguments_shouldThrowException() { @Test public void testBuilder_withTypicalData_shouldBuildCorrectAttributes() { - Notification notification = new Notification(startTime1, endTime1, + Notification notification = new Notification(STARTTIME_ONE, ENDTIME_ONE, NotificationType.TIPS, NotificationTargetUser.GENERAL, "Another tip for usage", "Here the message starts"); NotificationAttributes nfa = NotificationAttributes.valueOf(notification); - assertEquals(startTime1, nfa.getStartTime()); - assertEquals(endTime1, nfa.getEndTime()); + assertEquals(STARTTIME_ONE, nfa.getStartTime()); + assertEquals(ENDTIME_ONE, nfa.getEndTime()); assertEquals(NotificationType.TIPS, nfa.getType()); assertEquals(NotificationTargetUser.GENERAL, nfa.getTargetUser()); assertEquals("Another tip for usage", nfa.getTitle()); @@ -106,8 +106,8 @@ public void testBuilder_withTypicalData_shouldBuildCorrectAttributes() { @Test public void testCopyConstructor_shouldDoDeepCopyOfNotificationDetails() { NotificationAttributes nfa1 = NotificationAttributes.builder("notificationId") - .withStartTime(startTime1) - .withEndTime(endTime1) + .withStartTime(STARTTIME_ONE) + .withEndTime(ENDTIME_ONE) .withType(NotificationType.DEPRECATION) .withTargetUser(NotificationTargetUser.INSTRUCTOR) .withTitle("valid notification title") @@ -124,8 +124,8 @@ public void testCopyConstructor_shouldDoDeepCopyOfNotificationDetails() { public void testUpdateOptions_withTypicalUpdateOptions_shouldUpdateAttributeCorrectly() { NotificationAttributes.UpdateOptions updateOptions = NotificationAttributes.updateOptionsBuilder("notificationId") - .withStartTime(startTime2) - .withEndTime(endTime2) + .withStartTime(STARTTIME_TWO) + .withEndTime(ENDTIME_TWO) .withType(NotificationType.VERSION_NOTE) .withTargetUser(NotificationTargetUser.STUDENT) .withTitle("The edited title") @@ -136,8 +136,8 @@ public void testUpdateOptions_withTypicalUpdateOptions_shouldUpdateAttributeCorr NotificationAttributes notificationAttributes = NotificationAttributes.builder("notificationId") - .withStartTime(startTime1) - .withEndTime(endTime1) + .withStartTime(STARTTIME_ONE) + .withEndTime(ENDTIME_ONE) .withType(NotificationType.DEPRECATION) .withTargetUser(NotificationTargetUser.INSTRUCTOR) .withTitle("valid notification title") @@ -146,8 +146,8 @@ public void testUpdateOptions_withTypicalUpdateOptions_shouldUpdateAttributeCorr notificationAttributes.update(updateOptions); - assertEquals(startTime2, notificationAttributes.getStartTime()); - assertEquals(endTime2, notificationAttributes.getEndTime()); + assertEquals(STARTTIME_TWO, notificationAttributes.getStartTime()); + assertEquals(ENDTIME_TWO, notificationAttributes.getEndTime()); assertEquals(NotificationType.VERSION_NOTE, notificationAttributes.getType()); assertEquals(NotificationTargetUser.STUDENT, notificationAttributes.getTargetUser()); assertEquals("The edited title", notificationAttributes.getTitle()); @@ -182,8 +182,8 @@ public void testUpdateOptionsBuilder_withNullInput_shouldFailWithAssertionError( public void testEquals() { NotificationAttributes notificationAttributes = NotificationAttributes.builder("notificationId") - .withStartTime(startTime1) - .withEndTime(endTime1) + .withStartTime(STARTTIME_ONE) + .withEndTime(ENDTIME_ONE) .withType(NotificationType.DEPRECATION) .withTargetUser(NotificationTargetUser.INSTRUCTOR) .withTitle("valid notification title") @@ -198,8 +198,8 @@ public void testEquals() { // When the two notifications have same values but created at different time NotificationAttributes notificationAttributesSimilar = NotificationAttributes.builder("notificationId") - .withStartTime(startTime1) - .withEndTime(endTime1) + .withStartTime(STARTTIME_ONE) + .withEndTime(ENDTIME_ONE) .withType(NotificationType.DEPRECATION) .withTargetUser(NotificationTargetUser.INSTRUCTOR) .withTitle("valid notification title") @@ -210,8 +210,8 @@ public void testEquals() { NotificationAttributes notificationAttributesDifferent = NotificationAttributes.builder("differentId") - .withStartTime(startTime1) - .withEndTime(endTime1) + .withStartTime(STARTTIME_ONE) + .withEndTime(ENDTIME_ONE) .withType(NotificationType.DEPRECATION) .withTargetUser(NotificationTargetUser.INSTRUCTOR) .withTitle("valid notification title") @@ -228,8 +228,8 @@ public void testEquals() { public void testHashCode() { NotificationAttributes notificationAttributes = NotificationAttributes.builder("notificationId") - .withStartTime(startTime1) - .withEndTime(endTime1) + .withStartTime(STARTTIME_ONE) + .withEndTime(ENDTIME_ONE) .withType(NotificationType.DEPRECATION) .withTargetUser(NotificationTargetUser.INSTRUCTOR) .withTitle("valid notification title") @@ -244,8 +244,8 @@ public void testHashCode() { // When the two notifications have same values but created at different time NotificationAttributes notificationAttributesSimilar = NotificationAttributes.builder("notificationId") - .withStartTime(startTime1) - .withEndTime(endTime1) + .withStartTime(STARTTIME_ONE) + .withEndTime(ENDTIME_ONE) .withType(NotificationType.DEPRECATION) .withTargetUser(NotificationTargetUser.INSTRUCTOR) .withTitle("valid notification title") @@ -256,8 +256,8 @@ public void testHashCode() { NotificationAttributes notificationAttributesDifferent = NotificationAttributes.builder("differentId") - .withStartTime(startTime1) - .withEndTime(endTime1) + .withStartTime(STARTTIME_ONE) + .withEndTime(ENDTIME_ONE) .withType(NotificationType.DEPRECATION) .withTargetUser(NotificationTargetUser.INSTRUCTOR) .withTitle("valid notification title") diff --git a/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java b/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java index 4889e1b71b8..a6455c08eab 100644 --- a/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java +++ b/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java @@ -6,23 +6,18 @@ import teammates.common.datatransfer.NotificationTargetUser; import teammates.common.datatransfer.NotificationType; -import teammates.common.datatransfer.attributes.AccountAttributes; -import teammates.common.datatransfer.attributes.FeedbackSessionAttributes; import teammates.common.datatransfer.attributes.InstructorAttributes; import teammates.common.datatransfer.attributes.NotificationAttributes; -import teammates.common.datatransfer.attributes.StudentProfileAttributes; import teammates.common.util.Const; import teammates.ui.output.NotificationData; import teammates.ui.output.NotificationsData; -import teammates.ui.output.StudentProfileData; -import teammates.ui.request.Intent; /** * SUT: {@link GetNotificationAction}. */ public class GetNotificationActionTest extends BaseActionTest { - private NotificationAttributes notificationAttributes; + // TODO: add tests for isfetchingall @Override String getActionUri() { @@ -34,11 +29,6 @@ String getRequestMethod() { return GET; } - @Override - protected void prepareTestData() { - removeAndRestoreTypicalDataBundle(); - } - @Test @Override protected void testExecute() { @@ -46,35 +36,92 @@ protected void testExecute() { } @Override - protected void testAccessControl(){ + protected void testAccessControl() { verifyAnyUserCanAccess(); } @Test - public void testExecute_withFullDetailIntentForNonAdmin_shouldReturnDataWithFullDetail() { + public void testExecute_withFullDetailUserTypeForNonAdmin_shouldReturnDataWithFullDetail() { + int expectedNumberOfNotifications = 4; InstructorAttributes instructor = typicalBundle.instructors.get("instructor1OfCourse1"); loginAsInstructor(instructor.getGoogleId()); - NotificationAttributes notification = typicalBundle.notifications.get("notification-1"); + NotificationAttributes notification = typicalBundle.notifications.get("notification-5"); - String[] submissionParams = new String[] { + String[] requestParams = new String[] { Const.ParamsNames.NOTIFICATION_TARGET_USER, NotificationTargetUser.INSTRUCTOR.toString(), Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true), }; - GetNotificationAction action = getAction(submissionParams); + GetNotificationAction action = getAction(requestParams); JsonResult jsonResult = getJsonResult(action); NotificationsData output = (NotificationsData) jsonResult.getOutput(); List notifications = output.getNotifications(); - assertEquals(2, logic.getActiveNotificationsByTargetUser(NotificationTargetUser.INSTRUCTOR).size()); - assertEquals(2, notifications.size()); + assertEquals(expectedNumberOfNotifications, + logic.getActiveNotificationsByTargetUser(notification.getTargetUser()).size()); + assertEquals(expectedNumberOfNotifications, notifications.size()); NotificationData firstNotification = notifications.get(0); - assertEquals("notification-3", firstNotification.getNotificationId()); - assertEquals(NotificationType.VERSION_NOTE, firstNotification.getNotificationType()); + assertEquals("notification-5", firstNotification.getNotificationId()); + assertEquals(NotificationType.TIPS, firstNotification.getNotificationType()); assertEquals(NotificationTargetUser.INSTRUCTOR, firstNotification.getTargetUser()); - assertEquals("The first version note", firstNotification.getTitle()); - assertEquals("The version note content", firstNotification.getMessage()); + assertEquals("The first tip to instructor", firstNotification.getTitle()); + assertEquals("The first tip content", firstNotification.getMessage()); + } + + @Test + public void testExecute_withFullDetailUserTypeForAdmin_shouldReturnDataWithFullDetail() { + int expectedNumberOfNotifications = 4; + loginAsAdmin(); + NotificationAttributes notification = typicalBundle.notifications.get("notification-6"); + + String[] requestParams = new String[] { + Const.ParamsNames.NOTIFICATION_TARGET_USER, NotificationTargetUser.STUDENT.toString(), + Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true), + }; + + GetNotificationAction action = getAction(requestParams); + JsonResult jsonResult = getJsonResult(action); + + NotificationsData output = (NotificationsData) jsonResult.getOutput(); + List notifications = output.getNotifications(); + + assertEquals(expectedNumberOfNotifications, + logic.getActiveNotificationsByTargetUser(notification.getTargetUser()).size()); + assertEquals(expectedNumberOfNotifications, notifications.size()); + + NotificationData firstNotification = notifications.get(0); + assertEquals("notification-6", firstNotification.getNotificationId()); + assertEquals(NotificationType.DEPRECATION, firstNotification.getNotificationType()); + assertEquals(NotificationTargetUser.STUDENT, firstNotification.getTargetUser()); + assertEquals("The note of maintenance", firstNotification.getTitle()); + assertEquals("The content of maintenance", firstNotification.getMessage()); + } + + @Test + public void testExecute_withoutUserTypeForNonAdmin_shouldFail() { + InstructorAttributes instructor = typicalBundle.instructors.get("instructor1OfCourse1"); + loginAsInstructor(instructor.getGoogleId()); + GetNotificationAction action = getAction(Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true)); + assertThrows(AssertionError.class, action::execute); + } + + @Test + public void testExecute_invalidUserType_shouldFail() { + InstructorAttributes instructor = typicalBundle.instructors.get("instructor1OfCourse1"); + loginAsInstructor(instructor.getGoogleId()); + + // when usertype is GENERAL + verifyHttpParameterFailure(Const.ParamsNames.NOTIFICATION_TARGET_USER, + NotificationTargetUser.GENERAL.toString(), + Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, + String.valueOf(true)); + + // when usertype is a random string + verifyHttpParameterFailure(Const.ParamsNames.NOTIFICATION_TARGET_USER, + "invalid string", + Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, + String.valueOf(true)); } } diff --git a/src/test/resources/data/typicalDataBundle.json b/src/test/resources/data/typicalDataBundle.json index 8fe3a512849..b263135e57f 100644 --- a/src/test/resources/data/typicalDataBundle.json +++ b/src/test/resources/data/typicalDataBundle.json @@ -1906,6 +1906,42 @@ "title": "The first version note", "message": "The version note content", "shown": false + }, + "notification-4": { + "notificationId": "notification-4", + "startTime": "2011-04-04T00:00:00Z", + "endTime": "2024-04-04T00:00:00Z", + "createdAt": "2011-01-01T00:00:00Z", + "updatedAt": "2011-01-01T00:00:00Z", + "type": "MAINTENANCE", + "targetUser": "GENERAL", + "title": "The note of maintenance", + "message": "The content of maintenance", + "shown": false + }, + "notification-5": { + "notificationId": "notification-5", + "startTime": "2011-05-05T00:00:00Z", + "endTime": "2024-05-05T00:00:00Z", + "createdAt": "2011-01-01T00:00:00Z", + "updatedAt": "2011-01-01T00:00:00Z", + "type": "TIPS", + "targetUser": "INSTRUCTOR", + "title": "The first tip to instructor", + "message": "The first tip content", + "shown": false + }, + "notification-6": { + "notificationId": "notification-6", + "startTime": "2011-06-06T00:00:00Z", + "endTime": "2024-06-06T00:00:00Z", + "createdAt": "2011-01-01T00:00:00Z", + "updatedAt": "2011-01-01T00:00:00Z", + "type": "DEPRECATION", + "targetUser": "STUDENT", + "title": "The note of maintenance", + "message": "The content of maintenance", + "shown": false } } } From b268b5d319ac59d67d891e6ea12614b3a8c8c1f3 Mon Sep 17 00:00:00 2001 From: Zhang Ziqing Date: Mon, 14 Mar 2022 13:38:46 +0800 Subject: [PATCH 4/9] Add access control and its tests for GET api --- .../attributes/NotificationAttributes.java | 2 +- .../ui/webapi/GetNotificationAction.java | 21 +++++-- .../ui/webapi/GetNotificationActionTest.java | 58 ++++++++++++++++++- 3 files changed, 71 insertions(+), 10 deletions(-) diff --git a/src/main/java/teammates/common/datatransfer/attributes/NotificationAttributes.java b/src/main/java/teammates/common/datatransfer/attributes/NotificationAttributes.java index 5d405ff10e9..a6c98fdf5bc 100644 --- a/src/main/java/teammates/common/datatransfer/attributes/NotificationAttributes.java +++ b/src/main/java/teammates/common/datatransfer/attributes/NotificationAttributes.java @@ -160,7 +160,7 @@ public void setUpdatedAt(Instant updatedAt) { } /** - * Sorts the list of notifications by the start time, with the latest as the first. + * Sorts the list of notifications by the start time, with the latest as the first element. */ public static void sortByStartTime(List notifications) { notifications.sort(Comparator.comparing(NotificationAttributes::getStartTime)); diff --git a/src/main/java/teammates/ui/webapi/GetNotificationAction.java b/src/main/java/teammates/ui/webapi/GetNotificationAction.java index 1640636ff02..171e703a9d1 100644 --- a/src/main/java/teammates/ui/webapi/GetNotificationAction.java +++ b/src/main/java/teammates/ui/webapi/GetNotificationAction.java @@ -15,15 +15,28 @@ public class GetNotificationAction extends Action { private static final String INVALID_TARGET_USER = "Target user can only be STUDENT or INSTRUCTOR."; + private static final String UNAUTHORIZED_ACCESS = "You are not allowed to view this resource!"; @Override AuthType getMinAuthLevel() { - return AuthType.PUBLIC; + return AuthType.LOGGED_IN; } @Override void checkSpecificAccessControl() throws UnauthorizedAccessException { - // Any user can get notifications as long as its parameters are valid + if (userInfo.isAdmin) { + return; + } + String targetUserString = getRequestParamValue(Const.ParamsNames.NOTIFICATION_TARGET_USER); + String targetUserErrorMessage = FieldValidator.getInvalidityInfoForNotificationTargetUser(targetUserString); + if (!targetUserErrorMessage.isEmpty()) { + throw new InvalidHttpParameterException(targetUserErrorMessage); + } + NotificationTargetUser targetUser = NotificationTargetUser.valueOf(targetUserString); + if (targetUser == NotificationTargetUser.INSTRUCTOR && userInfo.isStudent + || targetUser == NotificationTargetUser.STUDENT && userInfo.isInstructor) { + throw new UnauthorizedAccessException(UNAUTHORIZED_ACCESS); + } } @Override @@ -46,10 +59,6 @@ public JsonResult execute() { if (targetUser == NotificationTargetUser.GENERAL) { throw new InvalidHttpParameterException(INVALID_TARGET_USER); } - if (targetUser == NotificationTargetUser.INSTRUCTOR && userInfo.isStudent - || targetUser == NotificationTargetUser.STUDENT && userInfo.isInstructor) { - throw new InvalidHttpParameterException(INVALID_TARGET_USER); - } notificationAttributes = logic.getActiveNotificationsByTargetUser(targetUser); } diff --git a/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java b/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java index a6455c08eab..959aeac87ed 100644 --- a/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java +++ b/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java @@ -8,6 +8,7 @@ import teammates.common.datatransfer.NotificationType; import teammates.common.datatransfer.attributes.InstructorAttributes; import teammates.common.datatransfer.attributes.NotificationAttributes; +import teammates.common.datatransfer.attributes.StudentAttributes; import teammates.common.util.Const; import teammates.ui.output.NotificationData; import teammates.ui.output.NotificationsData; @@ -35,13 +36,61 @@ protected void testExecute() { // See independent test cases } + @Test @Override protected void testAccessControl() { - verifyAnyUserCanAccess(); + InstructorAttributes instructor = typicalBundle.instructors.get("instructor1OfCourse1"); + loginAsInstructor(instructor.getGoogleId()); + ______TS("student notification not accessible to instructor"); + String[] requestParams = new String[] { + Const.ParamsNames.NOTIFICATION_TARGET_USER, NotificationTargetUser.STUDENT.toString(), + Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true), + }; + verifyCannotAccess(requestParams); + + ______TS("accessible to instructor"); + requestParams = new String[] { + Const.ParamsNames.NOTIFICATION_TARGET_USER, NotificationTargetUser.INSTRUCTOR.toString(), + Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true), + }; + verifyCanAccess(requestParams); + + ______TS("instructor notification not accessible to student"); + StudentAttributes studentAttributes = typicalBundle.students.get("student1InCourse1"); + loginAsStudent(studentAttributes.getGoogleId()); + requestParams = new String[] { + Const.ParamsNames.NOTIFICATION_TARGET_USER, NotificationTargetUser.INSTRUCTOR.toString(), + Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true), + }; + verifyCannotAccess(requestParams); + + ______TS("accessible to student"); + requestParams = new String[] { + Const.ParamsNames.NOTIFICATION_TARGET_USER, NotificationTargetUser.STUDENT.toString(), + Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true), + }; + verifyCanAccess(requestParams); + + ______TS("unknown target user"); + loginAsInstructor(instructor.getGoogleId()); + requestParams = new String[] { + Const.ParamsNames.NOTIFICATION_TARGET_USER, "unknown", + Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true), + }; + verifyHttpParameterFailureAcl(requestParams); + + ______TS("accessible to admin"); + loginAsAdmin(); + requestParams = new String[] { + Const.ParamsNames.NOTIFICATION_TARGET_USER, NotificationTargetUser.STUDENT.toString(), + Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true), + }; + verifyCanAccess(requestParams); } @Test - public void testExecute_withFullDetailUserTypeForNonAdmin_shouldReturnDataWithFullDetail() { + public void testExecute_withValidUserTypeForNonAdmin_shouldReturnData() { + ______TS("Request to fetch notification"); int expectedNumberOfNotifications = 4; InstructorAttributes instructor = typicalBundle.instructors.get("instructor1OfCourse1"); loginAsInstructor(instructor.getGoogleId()); @@ -71,7 +120,8 @@ public void testExecute_withFullDetailUserTypeForNonAdmin_shouldReturnDataWithFu } @Test - public void testExecute_withFullDetailUserTypeForAdmin_shouldReturnDataWithFullDetail() { + public void testExecute_withUserTypeForAdmin_shouldReturnData() { + ______TS("Admin request to fetch notification"); int expectedNumberOfNotifications = 4; loginAsAdmin(); NotificationAttributes notification = typicalBundle.notifications.get("notification-6"); @@ -101,6 +151,7 @@ public void testExecute_withFullDetailUserTypeForAdmin_shouldReturnDataWithFullD @Test public void testExecute_withoutUserTypeForNonAdmin_shouldFail() { + ______TS("Request without user type for non admin"); InstructorAttributes instructor = typicalBundle.instructors.get("instructor1OfCourse1"); loginAsInstructor(instructor.getGoogleId()); GetNotificationAction action = getAction(Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true)); @@ -109,6 +160,7 @@ public void testExecute_withoutUserTypeForNonAdmin_shouldFail() { @Test public void testExecute_invalidUserType_shouldFail() { + ______TS("Request without invalid user type"); InstructorAttributes instructor = typicalBundle.instructors.get("instructor1OfCourse1"); loginAsInstructor(instructor.getGoogleId()); From 8e132585654c347c39b2565d868d0ce2031d86ce Mon Sep 17 00:00:00 2001 From: Zhang Ziqing Date: Mon, 14 Mar 2022 13:53:50 +0800 Subject: [PATCH 5/9] Fix lint errors --- .../ui/webapi/GetNotificationActionTest.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java b/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java index 959aeac87ed..8a4d344b6cc 100644 --- a/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java +++ b/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java @@ -43,47 +43,47 @@ protected void testAccessControl() { loginAsInstructor(instructor.getGoogleId()); ______TS("student notification not accessible to instructor"); String[] requestParams = new String[] { - Const.ParamsNames.NOTIFICATION_TARGET_USER, NotificationTargetUser.STUDENT.toString(), - Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true), + Const.ParamsNames.NOTIFICATION_TARGET_USER, NotificationTargetUser.STUDENT.toString(), + Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true), }; verifyCannotAccess(requestParams); ______TS("accessible to instructor"); requestParams = new String[] { - Const.ParamsNames.NOTIFICATION_TARGET_USER, NotificationTargetUser.INSTRUCTOR.toString(), - Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true), + Const.ParamsNames.NOTIFICATION_TARGET_USER, NotificationTargetUser.INSTRUCTOR.toString(), + Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true), }; verifyCanAccess(requestParams); - + ______TS("instructor notification not accessible to student"); StudentAttributes studentAttributes = typicalBundle.students.get("student1InCourse1"); loginAsStudent(studentAttributes.getGoogleId()); requestParams = new String[] { - Const.ParamsNames.NOTIFICATION_TARGET_USER, NotificationTargetUser.INSTRUCTOR.toString(), - Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true), + Const.ParamsNames.NOTIFICATION_TARGET_USER, NotificationTargetUser.INSTRUCTOR.toString(), + Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true), }; verifyCannotAccess(requestParams); ______TS("accessible to student"); requestParams = new String[] { - Const.ParamsNames.NOTIFICATION_TARGET_USER, NotificationTargetUser.STUDENT.toString(), - Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true), + Const.ParamsNames.NOTIFICATION_TARGET_USER, NotificationTargetUser.STUDENT.toString(), + Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true), }; verifyCanAccess(requestParams); ______TS("unknown target user"); loginAsInstructor(instructor.getGoogleId()); requestParams = new String[] { - Const.ParamsNames.NOTIFICATION_TARGET_USER, "unknown", - Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true), + Const.ParamsNames.NOTIFICATION_TARGET_USER, "unknown", + Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true), }; verifyHttpParameterFailureAcl(requestParams); ______TS("accessible to admin"); loginAsAdmin(); requestParams = new String[] { - Const.ParamsNames.NOTIFICATION_TARGET_USER, NotificationTargetUser.STUDENT.toString(), - Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true), + Const.ParamsNames.NOTIFICATION_TARGET_USER, NotificationTargetUser.STUDENT.toString(), + Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true), }; verifyCanAccess(requestParams); } From 6be5200ad776c3e92b65af4406ff95e38e1e6742 Mon Sep 17 00:00:00 2001 From: Zhang Ziqing Date: Sun, 20 Mar 2022 20:16:21 +0800 Subject: [PATCH 6/9] Update data in dataBundle and add more abstractions --- .../NotificationAttributesTest.java | 1 - .../ui/webapi/GetNotificationActionTest.java | 36 +++++++++---------- .../resources/data/typicalDataBundle.json | 36 +++++++++---------- 3 files changed, 36 insertions(+), 37 deletions(-) diff --git a/src/test/java/teammates/common/datatransfer/attributes/NotificationAttributesTest.java b/src/test/java/teammates/common/datatransfer/attributes/NotificationAttributesTest.java index e7b918d702b..5bb5fbe5c95 100644 --- a/src/test/java/teammates/common/datatransfer/attributes/NotificationAttributesTest.java +++ b/src/test/java/teammates/common/datatransfer/attributes/NotificationAttributesTest.java @@ -266,5 +266,4 @@ public void testHashCode() { assertFalse(notificationAttributes.hashCode() == notificationAttributesDifferent.hashCode()); } - } diff --git a/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java b/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java index 8a4d344b6cc..5152fe16e88 100644 --- a/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java +++ b/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java @@ -10,6 +10,7 @@ import teammates.common.datatransfer.attributes.NotificationAttributes; import teammates.common.datatransfer.attributes.StudentAttributes; import teammates.common.util.Const; +import teammates.storage.entity.Notification; import teammates.ui.output.NotificationData; import teammates.ui.output.NotificationsData; @@ -82,7 +83,7 @@ protected void testAccessControl() { ______TS("accessible to admin"); loginAsAdmin(); requestParams = new String[] { - Const.ParamsNames.NOTIFICATION_TARGET_USER, NotificationTargetUser.STUDENT.toString(), + Const.ParamsNames.NOTIFICATION_TARGET_USER, null, Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true), }; verifyCanAccess(requestParams); @@ -94,7 +95,7 @@ public void testExecute_withValidUserTypeForNonAdmin_shouldReturnData() { int expectedNumberOfNotifications = 4; InstructorAttributes instructor = typicalBundle.instructors.get("instructor1OfCourse1"); loginAsInstructor(instructor.getGoogleId()); - NotificationAttributes notification = typicalBundle.notifications.get("notification-5"); + NotificationAttributes notification = typicalBundle.notifications.get("notification5"); String[] requestParams = new String[] { Const.ParamsNames.NOTIFICATION_TARGET_USER, NotificationTargetUser.INSTRUCTOR.toString(), @@ -112,22 +113,18 @@ public void testExecute_withValidUserTypeForNonAdmin_shouldReturnData() { assertEquals(expectedNumberOfNotifications, notifications.size()); NotificationData firstNotification = notifications.get(0); - assertEquals("notification-5", firstNotification.getNotificationId()); - assertEquals(NotificationType.TIPS, firstNotification.getNotificationType()); - assertEquals(NotificationTargetUser.INSTRUCTOR, firstNotification.getTargetUser()); - assertEquals("The first tip to instructor", firstNotification.getTitle()); - assertEquals("The first tip content", firstNotification.getMessage()); + verifyNotificationEquals(notification, firstNotification); } @Test - public void testExecute_withUserTypeForAdmin_shouldReturnData() { + public void testExecute_withoutUserTypeForAdmin_shouldReturnAllNotifications() { ______TS("Admin request to fetch notification"); - int expectedNumberOfNotifications = 4; + int expectedNumberOfNotifications = typicalBundle.notifications.size(); loginAsAdmin(); - NotificationAttributes notification = typicalBundle.notifications.get("notification-6"); + NotificationAttributes notification = typicalBundle.notifications.get("notification6"); String[] requestParams = new String[] { - Const.ParamsNames.NOTIFICATION_TARGET_USER, NotificationTargetUser.STUDENT.toString(), + Const.ParamsNames.NOTIFICATION_TARGET_USER, null, Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true), }; @@ -138,15 +135,11 @@ public void testExecute_withUserTypeForAdmin_shouldReturnData() { List notifications = output.getNotifications(); assertEquals(expectedNumberOfNotifications, - logic.getActiveNotificationsByTargetUser(notification.getTargetUser()).size()); + logic.getAllNotifications().size()); assertEquals(expectedNumberOfNotifications, notifications.size()); NotificationData firstNotification = notifications.get(0); - assertEquals("notification-6", firstNotification.getNotificationId()); - assertEquals(NotificationType.DEPRECATION, firstNotification.getNotificationType()); - assertEquals(NotificationTargetUser.STUDENT, firstNotification.getTargetUser()); - assertEquals("The note of maintenance", firstNotification.getTitle()); - assertEquals("The content of maintenance", firstNotification.getMessage()); + verifyNotificationEquals(notification, firstNotification); } @Test @@ -176,4 +169,11 @@ public void testExecute_invalidUserType_shouldFail() { Const.ParamsNames.NOTIFICATION_IS_FETCHING_ALL, String.valueOf(true)); } -} + + private void verifyNotificationEquals(NotificationAttributes expected, NotificationData actual) { + assertEquals(expected.getNotificationId(), actual.getNotificationId()); + assertEquals(expected.getType(), actual.getNotificationType()); + assertEquals(expected.getTargetUser(), actual.getTargetUser()); + assertEquals(expected.getTitle(), actual.getTitle()); + assertEquals(expected.getMessage(), actual.getMessage()); + }} diff --git a/src/test/resources/data/typicalDataBundle.json b/src/test/resources/data/typicalDataBundle.json index b263135e57f..6871d6f6ec2 100644 --- a/src/test/resources/data/typicalDataBundle.json +++ b/src/test/resources/data/typicalDataBundle.json @@ -1871,10 +1871,10 @@ } }, "notifications": { - "notification-1": { - "notificationId": "notification-1", + "notification1": { + "notificationId": "notification1", "startTime": "2011-01-01T00:00:00Z", - "endTime": "2024-01-01T00:00:00Z", + "endTime": "2099-01-01T00:00:00Z", "createdAt": "2011-01-01T00:00:00Z", "updatedAt": "2011-01-01T00:00:00Z", "type": "DEPRECATION", @@ -1883,10 +1883,10 @@ "message": "Deprecation happens in three minutes", "shown": false }, - "notification-2": { - "notificationId": "notification-2", + "notification2": { + "notificationId": "notification2", "startTime": "2011-02-02T00:00:00Z", - "endTime": "2024-02-02T00:00:00Z", + "endTime": "2099-02-02T00:00:00Z", "createdAt": "2011-01-01T00:00:00Z", "updatedAt": "2011-01-01T00:00:00Z", "type": "VERSION_NOTE", @@ -1895,10 +1895,10 @@ "message": "Exciting features", "shown": false }, - "notification-3": { - "notificationId": "notification-3", + "notification3": { + "notificationId": "notification3", "startTime": "2011-03-03T00:00:00Z", - "endTime": "2024-03-03T00:00:00Z", + "endTime": "2099-03-03T00:00:00Z", "createdAt": "2011-01-01T00:00:00Z", "updatedAt": "2011-01-01T00:00:00Z", "type": "VERSION_NOTE", @@ -1907,10 +1907,10 @@ "message": "The version note content", "shown": false }, - "notification-4": { - "notificationId": "notification-4", + "notification4": { + "notificationId": "notification4", "startTime": "2011-04-04T00:00:00Z", - "endTime": "2024-04-04T00:00:00Z", + "endTime": "2099-04-04T00:00:00Z", "createdAt": "2011-01-01T00:00:00Z", "updatedAt": "2011-01-01T00:00:00Z", "type": "MAINTENANCE", @@ -1919,10 +1919,10 @@ "message": "The content of maintenance", "shown": false }, - "notification-5": { - "notificationId": "notification-5", + "notification5": { + "notificationId": "notification5", "startTime": "2011-05-05T00:00:00Z", - "endTime": "2024-05-05T00:00:00Z", + "endTime": "2099-05-05T00:00:00Z", "createdAt": "2011-01-01T00:00:00Z", "updatedAt": "2011-01-01T00:00:00Z", "type": "TIPS", @@ -1931,10 +1931,10 @@ "message": "The first tip content", "shown": false }, - "notification-6": { - "notificationId": "notification-6", + "notification6": { + "notificationId": "notification6", "startTime": "2011-06-06T00:00:00Z", - "endTime": "2024-06-06T00:00:00Z", + "endTime": "2099-06-06T00:00:00Z", "createdAt": "2011-01-01T00:00:00Z", "updatedAt": "2011-01-01T00:00:00Z", "type": "DEPRECATION", From 7da538053016c4387ca542b91ebda7496c80cf7e Mon Sep 17 00:00:00 2001 From: Zhang Ziqing Date: Sun, 20 Mar 2022 22:14:22 +0800 Subject: [PATCH 7/9] Fix style --- .../java/teammates/ui/webapi/GetNotificationActionTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java b/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java index 5152fe16e88..a36ac57450f 100644 --- a/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java +++ b/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java @@ -5,12 +5,10 @@ import org.testng.annotations.Test; import teammates.common.datatransfer.NotificationTargetUser; -import teammates.common.datatransfer.NotificationType; import teammates.common.datatransfer.attributes.InstructorAttributes; import teammates.common.datatransfer.attributes.NotificationAttributes; import teammates.common.datatransfer.attributes.StudentAttributes; import teammates.common.util.Const; -import teammates.storage.entity.Notification; import teammates.ui.output.NotificationData; import teammates.ui.output.NotificationsData; @@ -176,4 +174,5 @@ private void verifyNotificationEquals(NotificationAttributes expected, Notificat assertEquals(expected.getTargetUser(), actual.getTargetUser()); assertEquals(expected.getTitle(), actual.getTitle()); assertEquals(expected.getMessage(), actual.getMessage()); - }} + } +} From c11b70a394f414942924f35296c997391237c9cd Mon Sep 17 00:00:00 2001 From: Zhang Ziqing Date: Tue, 22 Mar 2022 13:25:34 +0800 Subject: [PATCH 8/9] Refactor NotificationAttributesTest and add tests for FieldValidator --- .../NotificationAttributesTest.java | 138 +++++++----------- .../common/util/FieldValidatorTest.java | 77 ++++++++++ 2 files changed, 127 insertions(+), 88 deletions(-) diff --git a/src/test/java/teammates/common/datatransfer/attributes/NotificationAttributesTest.java b/src/test/java/teammates/common/datatransfer/attributes/NotificationAttributesTest.java index 5bb5fbe5c95..585f187b46e 100644 --- a/src/test/java/teammates/common/datatransfer/attributes/NotificationAttributesTest.java +++ b/src/test/java/teammates/common/datatransfer/attributes/NotificationAttributesTest.java @@ -13,29 +13,15 @@ * SUT: {@link NotificationAttributes}. */ public class NotificationAttributesTest extends BaseTestCase { - private static final Instant STARTTIME_ONE = Instant.now().plusSeconds(3600); - private static final Instant ENDTIME_ONE = Instant.now().plusSeconds(7200); - private static final Instant STARTTIME_TWO = Instant.now().plusSeconds(1000); - private static final Instant ENDTIME_TWO = Instant.now().plusSeconds(10000); - @Test public void testValueOf_withAllFieldPopulatedNotificationAttributes_shouldGenerateAttributesCorrectly() { Notification notification = new Notification("valid-notification-id", - STARTTIME_ONE, ENDTIME_ONE, + Instant.now().plusSeconds(3600), Instant.now().plusSeconds(7200), NotificationType.DEPRECATION, NotificationTargetUser.INSTRUCTOR, "valid notification title", "valid notification message", false, Instant.now(), Instant.now()); NotificationAttributes nfa = NotificationAttributes.valueOf(notification); - assertEquals(notification.getNotificationId(), nfa.getNotificationId()); - assertEquals(notification.getStartTime(), nfa.getStartTime()); - assertEquals(notification.getEndTime(), nfa.getEndTime()); - assertEquals(notification.getType(), nfa.getType()); - assertEquals(notification.getTargetUser(), nfa.getTargetUser()); - assertEquals(notification.getTitle(), nfa.getTitle()); - assertEquals(notification.getMessage(), nfa.getMessage()); - assertEquals(notification.isShown(), nfa.isShown()); - assertEquals(notification.getCreatedAt(), nfa.getCreatedAt()); - assertEquals(notification.getUpdatedAt(), nfa.getUpdatedAt()); + verifyNotificationEquals(nfa, notification); } @Test @@ -91,29 +77,21 @@ public void testBuilder_withNullArguments_shouldThrowException() { @Test public void testBuilder_withTypicalData_shouldBuildCorrectAttributes() { - Notification notification = new Notification(STARTTIME_ONE, ENDTIME_ONE, - NotificationType.TIPS, NotificationTargetUser.GENERAL, - "Another tip for usage", "Here the message starts"); - NotificationAttributes nfa = NotificationAttributes.valueOf(notification); - assertEquals(STARTTIME_ONE, nfa.getStartTime()); - assertEquals(ENDTIME_ONE, nfa.getEndTime()); - assertEquals(NotificationType.TIPS, nfa.getType()); - assertEquals(NotificationTargetUser.GENERAL, nfa.getTargetUser()); - assertEquals("Another tip for usage", nfa.getTitle()); - assertEquals("Here the message starts", nfa.getMessage()); + NotificationAttributes nfa = generateTypicalNotificationAttributesObject(); + assertEquals("notificationId", nfa.getNotificationId()); + assertEquals(Instant.ofEpochSecond(1234567890), nfa.getStartTime()); + assertEquals(Instant.ofEpochSecond(1234567890).plusSeconds(7200), nfa.getEndTime()); + assertEquals(NotificationType.DEPRECATION, nfa.getType()); + assertEquals(NotificationTargetUser.INSTRUCTOR, nfa.getTargetUser()); + assertEquals("valid notification title", nfa.getTitle()); + assertEquals("valid message", nfa.getMessage()); } @Test public void testCopyConstructor_shouldDoDeepCopyOfNotificationDetails() { - NotificationAttributes nfa1 = NotificationAttributes.builder("notificationId") - .withStartTime(STARTTIME_ONE) - .withEndTime(ENDTIME_ONE) - .withType(NotificationType.DEPRECATION) - .withTargetUser(NotificationTargetUser.INSTRUCTOR) - .withTitle("valid notification title") - .withMessage("The first message") - .build(); + NotificationAttributes nfa1 = generateTypicalNotificationAttributesObject(); NotificationAttributes nfa2 = nfa1.getCopy(); + nfa1.setMessage("The first message"); nfa2.setMessage("The second message"); assertEquals(nfa1.getMessage(), "The first message"); @@ -124,8 +102,8 @@ public void testCopyConstructor_shouldDoDeepCopyOfNotificationDetails() { public void testUpdateOptions_withTypicalUpdateOptions_shouldUpdateAttributeCorrectly() { NotificationAttributes.UpdateOptions updateOptions = NotificationAttributes.updateOptionsBuilder("notificationId") - .withStartTime(STARTTIME_TWO) - .withEndTime(ENDTIME_TWO) + .withStartTime(Instant.ofEpochSecond(1234567890).plusSeconds(1000)) + .withEndTime(Instant.ofEpochSecond(1234567890).plusSeconds(10000)) .withType(NotificationType.VERSION_NOTE) .withTargetUser(NotificationTargetUser.STUDENT) .withTitle("The edited title") @@ -134,20 +112,11 @@ public void testUpdateOptions_withTypicalUpdateOptions_shouldUpdateAttributeCorr assertEquals("notificationId", updateOptions.getNotificationId()); - NotificationAttributes notificationAttributes = - NotificationAttributes.builder("notificationId") - .withStartTime(STARTTIME_ONE) - .withEndTime(ENDTIME_ONE) - .withType(NotificationType.DEPRECATION) - .withTargetUser(NotificationTargetUser.INSTRUCTOR) - .withTitle("valid notification title") - .withMessage("The first message") - .build(); - + NotificationAttributes notificationAttributes = generateTypicalNotificationAttributesObject(); notificationAttributes.update(updateOptions); - assertEquals(STARTTIME_TWO, notificationAttributes.getStartTime()); - assertEquals(ENDTIME_TWO, notificationAttributes.getEndTime()); + assertEquals(Instant.ofEpochSecond(1234567890).plusSeconds(1000), notificationAttributes.getStartTime()); + assertEquals(Instant.ofEpochSecond(1234567890).plusSeconds(10000), notificationAttributes.getEndTime()); assertEquals(NotificationType.VERSION_NOTE, notificationAttributes.getType()); assertEquals(NotificationTargetUser.STUDENT, notificationAttributes.getTargetUser()); assertEquals("The edited title", notificationAttributes.getTitle()); @@ -180,15 +149,7 @@ public void testUpdateOptionsBuilder_withNullInput_shouldFailWithAssertionError( @Test public void testEquals() { - NotificationAttributes notificationAttributes = - NotificationAttributes.builder("notificationId") - .withStartTime(STARTTIME_ONE) - .withEndTime(ENDTIME_ONE) - .withType(NotificationType.DEPRECATION) - .withTargetUser(NotificationTargetUser.INSTRUCTOR) - .withTitle("valid notification title") - .withMessage("valid message") - .build(); + NotificationAttributes notificationAttributes = generateTypicalNotificationAttributesObject(); // When the two notifications are exact copies NotificationAttributes notificationAttributesCopy = notificationAttributes.getCopy(); @@ -196,22 +157,14 @@ public void testEquals() { assertTrue(notificationAttributes.equals(notificationAttributesCopy)); // When the two notifications have same values but created at different time - NotificationAttributes notificationAttributesSimilar = - NotificationAttributes.builder("notificationId") - .withStartTime(STARTTIME_ONE) - .withEndTime(ENDTIME_ONE) - .withType(NotificationType.DEPRECATION) - .withTargetUser(NotificationTargetUser.INSTRUCTOR) - .withTitle("valid notification title") - .withMessage("valid message") - .build(); + NotificationAttributes notificationAttributesSimilar = generateTypicalNotificationAttributesObject(); assertTrue(notificationAttributes.equals(notificationAttributesSimilar)); NotificationAttributes notificationAttributesDifferent = NotificationAttributes.builder("differentId") - .withStartTime(STARTTIME_ONE) - .withEndTime(ENDTIME_ONE) + .withStartTime(Instant.ofEpochSecond(1234567890)) + .withEndTime(Instant.ofEpochSecond(1234567890).plusSeconds(7200)) .withType(NotificationType.DEPRECATION) .withTargetUser(NotificationTargetUser.INSTRUCTOR) .withTitle("valid notification title") @@ -226,15 +179,7 @@ public void testEquals() { @Test public void testHashCode() { - NotificationAttributes notificationAttributes = - NotificationAttributes.builder("notificationId") - .withStartTime(STARTTIME_ONE) - .withEndTime(ENDTIME_ONE) - .withType(NotificationType.DEPRECATION) - .withTargetUser(NotificationTargetUser.INSTRUCTOR) - .withTitle("valid notification title") - .withMessage("valid message") - .build(); + NotificationAttributes notificationAttributes = generateTypicalNotificationAttributesObject(); // When the two notifications are exact copies NotificationAttributes notificationAttributesCopy = notificationAttributes.getCopy(); @@ -242,22 +187,15 @@ public void testHashCode() { assertTrue(notificationAttributes.hashCode() == notificationAttributesCopy.hashCode()); // When the two notifications have same values but created at different time - NotificationAttributes notificationAttributesSimilar = - NotificationAttributes.builder("notificationId") - .withStartTime(STARTTIME_ONE) - .withEndTime(ENDTIME_ONE) - .withType(NotificationType.DEPRECATION) - .withTargetUser(NotificationTargetUser.INSTRUCTOR) - .withTitle("valid notification title") - .withMessage("valid message") - .build(); + NotificationAttributes notificationAttributesSimilar = generateTypicalNotificationAttributesObject(); assertTrue(notificationAttributes.hashCode() == notificationAttributesSimilar.hashCode()); + // notification attributes with a different id. NotificationAttributes notificationAttributesDifferent = NotificationAttributes.builder("differentId") - .withStartTime(STARTTIME_ONE) - .withEndTime(ENDTIME_ONE) + .withStartTime(Instant.ofEpochSecond(1234567890)) + .withEndTime(Instant.ofEpochSecond(1234567890).plusSeconds(7200)) .withType(NotificationType.DEPRECATION) .withTargetUser(NotificationTargetUser.INSTRUCTOR) .withTitle("valid notification title") @@ -266,4 +204,28 @@ public void testHashCode() { assertFalse(notificationAttributes.hashCode() == notificationAttributesDifferent.hashCode()); } + + private NotificationAttributes generateTypicalNotificationAttributesObject() { + return NotificationAttributes.builder("notificationId") + .withStartTime(Instant.ofEpochSecond(1234567890)) + .withEndTime(Instant.ofEpochSecond(1234567890).plusSeconds(7200)) + .withType(NotificationType.DEPRECATION) + .withTargetUser(NotificationTargetUser.INSTRUCTOR) + .withTitle("valid notification title") + .withMessage("valid message") + .build(); + } + + private void verifyNotificationEquals(NotificationAttributes nfa, Notification notification) { + assertEquals(notification.getNotificationId(), nfa.getNotificationId()); + assertEquals(notification.getStartTime(), nfa.getStartTime()); + assertEquals(notification.getEndTime(), nfa.getEndTime()); + assertEquals(notification.getType(), nfa.getType()); + assertEquals(notification.getTargetUser(), nfa.getTargetUser()); + assertEquals(notification.getTitle(), nfa.getTitle()); + assertEquals(notification.getMessage(), nfa.getMessage()); + assertEquals(notification.isShown(), nfa.isShown()); + assertEquals(notification.getCreatedAt(), nfa.getCreatedAt()); + assertEquals(notification.getUpdatedAt(), nfa.getUpdatedAt()); + } } diff --git a/src/test/java/teammates/common/util/FieldValidatorTest.java b/src/test/java/teammates/common/util/FieldValidatorTest.java index c0f308cf236..6f5a63fbf03 100644 --- a/src/test/java/teammates/common/util/FieldValidatorTest.java +++ b/src/test/java/teammates/common/util/FieldValidatorTest.java @@ -589,6 +589,83 @@ public void testGetInvalidityInfoForTimeForVisibilityStartAndResultsPublish_inva visibilityStart, resultsPublish)); } + @Test + public void testGetInvalidityInfoForTimeForNotificationStartAndEnd_valid_returnEmptyString() { + Instant notificationStart = TimeHelperExtension.getInstantHoursOffsetFromNow(-1); + Instant notificationEnd = TimeHelperExtension.getInstantHoursOffsetFromNow(1); + assertEquals("", + FieldValidator.getInvalidityInfoForTimeForNotificationStartAndEnd( + notificationStart, notificationEnd)); + } + + @Test + public void testGetInvalidityInfoForTimeForNotificationStartAndEnd_inValid_returnErrorString() { + Instant notificationStart = TimeHelperExtension.getInstantHoursOffsetFromNow(1); + Instant notificationEnd = TimeHelperExtension.getInstantHoursOffsetFromNow(-1); + assertEquals("The time when the notification will expire for this notification cannot be earlier " + + "than the time when the notification will be visible.", + FieldValidator.getInvalidityInfoForTimeForNotificationStartAndEnd( + notificationStart, notificationEnd)); + } + + @Test + public void testGetInvalidityInfoForNotificationTitle_valid_returnEmptyString() { + String title = "valid title"; + assertEquals("", FieldValidator.getInvalidityInfoForNotificationTitle(title)); + } + + @Test + public void testGetInvalidityInfoForNotificationTitle_inValid_returnErrorString() { + assertEquals("The field 'notification title' is empty.", + FieldValidator.getInvalidityInfoForNotificationTitle("")); + String invalidNotificationTitle = StringHelperExtension.generateStringOfLength( + FieldValidator.NOTIFICATION_TITLE_MAX_LENGTH + 1); + assertEquals("\"" + invalidNotificationTitle + "\" is not acceptable to TEAMMATES as a/an " + + "notification title because it is too long. " + + "The value of a/an notification title should be no longer than " + + FieldValidator.NOTIFICATION_TITLE_MAX_LENGTH + + " characters. It should not be empty.", + FieldValidator.getInvalidityInfoForNotificationTitle(invalidNotificationTitle)); + } + + @Test + public void testGetInvalidityInfoForNotificationBody_valid_returnEmptyString() { + String body = "valid body"; + assertEquals("", FieldValidator.getInvalidityInfoForNotificationBody(body)); + } + + @Test + public void testGetInvalidityInfoForNotificationBody_inValid_returnErrorString() { + assertEquals("The field 'notification message' is empty.", + FieldValidator.getInvalidityInfoForNotificationBody("")); + } + + @Test + public void testGetInvalidityInfoForNotificationType_valid_returnEmptyString() { + String notificationType = "DEPRECATION"; + assertEquals("", FieldValidator.getInvalidityInfoForNotificationType(notificationType)); + } + + @Test + public void testGetInvalidityInfoForNotificationType_inValid_returnErrorString() { + String notificationType = "invalid type"; + assertEquals("\"invalid type\" is not an accepted notification type to TEAMMATES. ", + FieldValidator.getInvalidityInfoForNotificationType(notificationType)); + } + + @Test + public void testGetInvalidityInfoForNotificationTargetUser_valid_returnEmptyString() { + String user = "GENERAL"; + assertEquals("", FieldValidator.getInvalidityInfoForNotificationTargetUser(user)); + } + + @Test + public void testGetInvalidityInfoForNotificationTargetUser_inValid_returnErrorString() { + String user = "invalid user"; + assertEquals("\"invalid user\" is not an accepted notification target user to TEAMMATES. ", + FieldValidator.getInvalidityInfoForNotificationTargetUser(user)); + } + @Test public void testRegexName() { ______TS("success: typical name"); From a33ab4e76d2bc91ec37ce024d2b410ac782c6a7b Mon Sep 17 00:00:00 2001 From: Zhang Ziqing Date: Tue, 22 Mar 2022 21:12:36 +0800 Subject: [PATCH 9/9] Use variable in string concatenation and update expected number --- .../common/util/FieldValidatorTest.java | 16 ++++++---------- .../ui/webapi/GetNotificationActionTest.java | 5 ++--- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/src/test/java/teammates/common/util/FieldValidatorTest.java b/src/test/java/teammates/common/util/FieldValidatorTest.java index 6f5a63fbf03..ba885686619 100644 --- a/src/test/java/teammates/common/util/FieldValidatorTest.java +++ b/src/test/java/teammates/common/util/FieldValidatorTest.java @@ -610,8 +610,7 @@ public void testGetInvalidityInfoForTimeForNotificationStartAndEnd_inValid_retur @Test public void testGetInvalidityInfoForNotificationTitle_valid_returnEmptyString() { - String title = "valid title"; - assertEquals("", FieldValidator.getInvalidityInfoForNotificationTitle(title)); + assertEquals("", FieldValidator.getInvalidityInfoForNotificationTitle("valid title")); } @Test @@ -630,8 +629,7 @@ public void testGetInvalidityInfoForNotificationTitle_inValid_returnErrorString( @Test public void testGetInvalidityInfoForNotificationBody_valid_returnEmptyString() { - String body = "valid body"; - assertEquals("", FieldValidator.getInvalidityInfoForNotificationBody(body)); + assertEquals("", FieldValidator.getInvalidityInfoForNotificationBody("valid body")); } @Test @@ -642,27 +640,25 @@ public void testGetInvalidityInfoForNotificationBody_inValid_returnErrorString() @Test public void testGetInvalidityInfoForNotificationType_valid_returnEmptyString() { - String notificationType = "DEPRECATION"; - assertEquals("", FieldValidator.getInvalidityInfoForNotificationType(notificationType)); + assertEquals("", FieldValidator.getInvalidityInfoForNotificationType("DEPRECATION")); } @Test public void testGetInvalidityInfoForNotificationType_inValid_returnErrorString() { String notificationType = "invalid type"; - assertEquals("\"invalid type\" is not an accepted notification type to TEAMMATES. ", + assertEquals("\"" + notificationType + "\" is not an accepted notification type to TEAMMATES. ", FieldValidator.getInvalidityInfoForNotificationType(notificationType)); } @Test public void testGetInvalidityInfoForNotificationTargetUser_valid_returnEmptyString() { - String user = "GENERAL"; - assertEquals("", FieldValidator.getInvalidityInfoForNotificationTargetUser(user)); + assertEquals("", FieldValidator.getInvalidityInfoForNotificationTargetUser("GENERAL")); } @Test public void testGetInvalidityInfoForNotificationTargetUser_inValid_returnErrorString() { String user = "invalid user"; - assertEquals("\"invalid user\" is not an accepted notification target user to TEAMMATES. ", + assertEquals("\"" + user + "\" is not an accepted notification target user to TEAMMATES. ", FieldValidator.getInvalidityInfoForNotificationTargetUser(user)); } diff --git a/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java b/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java index a36ac57450f..3f76b69b6b4 100644 --- a/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java +++ b/src/test/java/teammates/ui/webapi/GetNotificationActionTest.java @@ -90,10 +90,11 @@ protected void testAccessControl() { @Test public void testExecute_withValidUserTypeForNonAdmin_shouldReturnData() { ______TS("Request to fetch notification"); - int expectedNumberOfNotifications = 4; InstructorAttributes instructor = typicalBundle.instructors.get("instructor1OfCourse1"); loginAsInstructor(instructor.getGoogleId()); NotificationAttributes notification = typicalBundle.notifications.get("notification5"); + int expectedNumberOfNotifications = + logic.getActiveNotificationsByTargetUser(notification.getTargetUser()).size(); String[] requestParams = new String[] { Const.ParamsNames.NOTIFICATION_TARGET_USER, NotificationTargetUser.INSTRUCTOR.toString(), @@ -106,8 +107,6 @@ public void testExecute_withValidUserTypeForNonAdmin_shouldReturnData() { NotificationsData output = (NotificationsData) jsonResult.getOutput(); List notifications = output.getNotifications(); - assertEquals(expectedNumberOfNotifications, - logic.getActiveNotificationsByTargetUser(notification.getTargetUser()).size()); assertEquals(expectedNumberOfNotifications, notifications.size()); NotificationData firstNotification = notifications.get(0);