diff --git a/src/main/java/org/opentripplanner/middleware/models/TripMonitorNotification.java b/src/main/java/org/opentripplanner/middleware/models/TripMonitorNotification.java index 4009d6707..5daf1ced0 100644 --- a/src/main/java/org/opentripplanner/middleware/models/TripMonitorNotification.java +++ b/src/main/java/org/opentripplanner/middleware/models/TripMonitorNotification.java @@ -16,9 +16,19 @@ */ public class TripMonitorNotification extends Model { private static final Logger LOG = LoggerFactory.getLogger(TripMonitorNotification.class); + public final NotificationType type; public final String body; + /** Getter functions are used by HTML template renderer */ + public String getBody() { + return body; + } + + public NotificationType getType() { + return type; + } + public TripMonitorNotification(NotificationType type, String body) { this.type = type; this.body = body; diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java index 8e26f39a3..0e90e56c0 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java @@ -428,11 +428,7 @@ private void sendNotifications() { Map templateData = Map.of( "tripId", trip.id, "tripName", trip.tripName, - "notifications", notifications.stream() - // Make certain notifications, such as the initial reminder one, appear on top. - .sorted(Comparator.comparingInt(TripMonitorNotification::sortOrder)) - .map(notification -> notification.body) - .collect(Collectors.toList()) + "notifications", new ArrayList<>(notifications) ); // FIXME: Change log level LOG.info("Sending notification to user {}", trip.userId); diff --git a/src/main/resources/templates/MonitoredTripHtml.ftl b/src/main/resources/templates/MonitoredTripHtml.ftl index 1876a405b..444e50a6a 100644 --- a/src/main/resources/templates/MonitoredTripHtml.ftl +++ b/src/main/resources/templates/MonitoredTripHtml.ftl @@ -8,11 +8,21 @@ <#macro EmailMain>

Your trip has the following notifications:

-
diff --git a/src/main/resources/templates/MonitoredTripPush.ftl b/src/main/resources/templates/MonitoredTripPush.ftl index 070f3cf68..7d923e19e 100644 --- a/src/main/resources/templates/MonitoredTripPush.ftl +++ b/src/main/resources/templates/MonitoredTripPush.ftl @@ -5,7 +5,14 @@ - iOS: 178 characters over up to 4 lines, - Android: 240 characters (We are not using notification title at this time). The max length is thus 178 characters. --->${tripName} + - List alerts with bullets if there are more than one of them. +--> +<#if notifications?size gt 1> <#list notifications as notification> -• ${notification} - \ No newline at end of file +• ${notification.body} + +<#else> +<#list notifications as notification> +${notification.body} + + \ No newline at end of file diff --git a/src/main/resources/templates/MonitoredTripSms.ftl b/src/main/resources/templates/MonitoredTripSms.ftl index 892922329..b20176f9d 100644 --- a/src/main/resources/templates/MonitoredTripSms.ftl +++ b/src/main/resources/templates/MonitoredTripSms.ftl @@ -4,9 +4,16 @@ --> Your trip has the following notifications: + +<#if notifications?size gt 1> +<#list notifications as notification> +• ${notification.body} + +<#else> <#list notifications as notification> - -${notification} +${notification.body} + View trip: ${OTP_UI_URL}${TRIPS_PATH}/${tripId} diff --git a/src/main/resources/templates/MonitoredTripText.ftl b/src/main/resources/templates/MonitoredTripText.ftl index 0c7358339..6e5641622 100644 --- a/src/main/resources/templates/MonitoredTripText.ftl +++ b/src/main/resources/templates/MonitoredTripText.ftl @@ -4,8 +4,14 @@ --> Your trip has the following notifications: -<#list notifications as notification> - -${notification} - +<#if notifications?size gt 1> + <#list notifications as notification> + • ${notification.body} + +<#else> + <#list notifications as notification> + ${notification.body} + + View trip: ${OTP_UI_URL}${TRIPS_PATH}/${tripId} \ No newline at end of file diff --git a/src/test/java/org/opentripplanner/middleware/utils/TemplateUtilsTest.java b/src/test/java/org/opentripplanner/middleware/utils/TemplateUtilsTest.java index a1b6a73b8..020002a18 100644 --- a/src/test/java/org/opentripplanner/middleware/utils/TemplateUtilsTest.java +++ b/src/test/java/org/opentripplanner/middleware/utils/TemplateUtilsTest.java @@ -3,7 +3,9 @@ import org.hamcrest.MatcherAssert; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; +import org.opentripplanner.middleware.models.TripMonitorNotification; import org.opentripplanner.middleware.testutils.OtpMiddlewareTestEnvironment; +import org.opentripplanner.middleware.tripmonitor.jobs.NotificationType; import java.util.ArrayList; import java.util.HashSet; @@ -14,6 +16,14 @@ import static com.zenika.snapshotmatcher.SnapshotMatcher.matchesSnapshot; import static org.hamcrest.MatcherAssert.assertThat; +/** + * Unit tests for {@code resources/templates/*.ftl} Freemarker Template Language files. + *

+ * When the tests are first run, the {@code com.zenika.snapshotmatcher.SnapshotMatcher} class will create JSON snapshot + * files in {@code resources/snapshots/org/opentripplanner/middleware} which are used thereafter to compare with the + * results of the template files. If you change any {@code *.ftl} files you will need to delete their corresponding + * snapshot files, run the tests to create new snapshots, and then commit those files along with new template files. + */ public class TemplateUtilsTest extends OtpMiddlewareTestEnvironment { /** * A parameterized test that checks whether various templates render in a way that matches a snapshot. The name of @@ -50,8 +60,10 @@ public static List createTemplateRenderingTestCases() // Trip Monitor Notifications tests (for CheckMonitoredTrip#sendNotifications). Map notificationsData = Map.of( "tripId", "18f642d5-f7a8-475a-9469-800129e6c0b3", - "notifications", List.of("Test notification.", "Another notification.") - ); + "notifications", List.of( + new TripMonitorNotification(NotificationType.INITIAL_REMINDER, "This is the initial reminder text"), + new TripMonitorNotification(NotificationType.DEPARTURE_DELAY, "This is the departure delay text") + )); testCases.add(new TemplateRenderingTestCase( notificationsData, "MonitoredTripSms.ftl", "Monitored Trip SMS" )); diff --git a/src/test/resources/snapshots/org/opentripplanner/middleware/utils/TemplateUtilsTest/Monitored_Trip_HTML_Email-0.json b/src/test/resources/snapshots/org/opentripplanner/middleware/utils/TemplateUtilsTest/Monitored_Trip_HTML_Email-0.json index 3a22462f5..4c1dcc9be 100644 --- a/src/test/resources/snapshots/org/opentripplanner/middleware/utils/TemplateUtilsTest/Monitored_Trip_HTML_Email-0.json +++ b/src/test/resources/snapshots/org/opentripplanner/middleware/utils/TemplateUtilsTest/Monitored_Trip_HTML_Email-0.json @@ -1 +1 @@ -"\n\n\n\n\n\n

\n
\n
\n

Your trip has the following notifications:

\n
    \n
  • Test notification.
    • \n
  • Another notification.
    • \n
\n

View all of your saved trips in Trip Planner.

\n
\n
\n
\n
\n

\n \n You're receiving this email because you're subscribed to notifications through Trip Planner. You can manage that subscription here.\n \n

\n
\n
\n\n" \ No newline at end of file +"\n\n\n\n\n\n
\n
\n
\n

Your trip has the following notifications:

\n\n

This is the initial reminder text

\n\n
    \n
  • This is the departure delay text
    • \n
\n\n

View all of your saved trips in Trip Planner.

\n
\n
\n
\n
\n

\n \n You're receiving this email because you're subscribed to notifications through Trip Planner. You can manage that subscription here.\n \n

\n
\n
\n\n" \ No newline at end of file diff --git a/src/test/resources/snapshots/org/opentripplanner/middleware/utils/TemplateUtilsTest/Monitored_Trip_SMS-0.json b/src/test/resources/snapshots/org/opentripplanner/middleware/utils/TemplateUtilsTest/Monitored_Trip_SMS-0.json index cca100bf4..dd46a7961 100644 --- a/src/test/resources/snapshots/org/opentripplanner/middleware/utils/TemplateUtilsTest/Monitored_Trip_SMS-0.json +++ b/src/test/resources/snapshots/org/opentripplanner/middleware/utils/TemplateUtilsTest/Monitored_Trip_SMS-0.json @@ -1 +1 @@ -"\nYour trip has the following notifications:\n -Test notification.\n -Another notification.\n\nView trip: https://plan.example.com/#/account/trips/18f642d5-f7a8-475a-9469-800129e6c0b3\n\nTo stop receiving notifications, reply STOP." \ No newline at end of file +"\nYour trip has the following notifications:\n\n• This is the initial reminder text\n• This is the departure delay text\n\nView trip: https://plan.example.com/#/account/trips/18f642d5-f7a8-475a-9469-800129e6c0b3\n\nTo stop receiving notifications, reply STOP." \ No newline at end of file diff --git a/src/test/resources/snapshots/org/opentripplanner/middleware/utils/TemplateUtilsTest/Monitored_Trip_Text_Email-0.json b/src/test/resources/snapshots/org/opentripplanner/middleware/utils/TemplateUtilsTest/Monitored_Trip_Text_Email-0.json index aa69f8dbe..df4c3dc97 100644 --- a/src/test/resources/snapshots/org/opentripplanner/middleware/utils/TemplateUtilsTest/Monitored_Trip_Text_Email-0.json +++ b/src/test/resources/snapshots/org/opentripplanner/middleware/utils/TemplateUtilsTest/Monitored_Trip_Text_Email-0.json @@ -1 +1 @@ -"\nYour trip has the following notifications:\n -Test notification.\n -Another notification.\n\nView trip: https://plan.example.com/#/account/trips/18f642d5-f7a8-475a-9469-800129e6c0b3" \ No newline at end of file +"\nYour trip has the following notifications:\n • This is the initial reminder text\n • This is the departure delay text\n\nView trip: https://plan.example.com/#/account/trips/18f642d5-f7a8-475a-9469-800129e6c0b3" \ No newline at end of file