Merge pull request #196 from ibi-group/push-notif-message

Improve trip alert template (push + email)

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;

src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java

@@ -436,11 +436,7 @@ private void sendNotifications() {
         Map<String, Object> 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);

src/main/resources/templates/MonitoredTripHtml.ftl

@@ -8,11 +8,21 @@
 <#macro EmailMain>
     <div>
         <h1>Your trip has the following notifications:</h1>
-        <ul>
-        <#list notifications as notification>
-          <li>${notification}</li>
+
+         <#list notifications as notification>
+            <#if notification.type == "INITIAL_REMINDER">
+                <p>${notification.body}</p>
+            </#if>
         </#list>
+
+        <ul>
+            <#list notifications as notification>
+                <#if notification.type != "INITIAL_REMINDER">
+                    <li>${notification.body}</li>
+                </#if>
+            </#list>
         </ul>
+
         <p>View all of your saved trips in <a href="${OTP_UI_URL}${TRIPS_PATH}/${tripId}">${OTP_UI_NAME}</a>.</p>
     </div>
 </#macro>

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}
-</#list>
+• ${notification.body}
+</#list>
+<#else>
+<#list notifications as notification>
+${notification.body}
+</#list>
+</#if>

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}
+</#list>
+<#else>
 <#list notifications as notification>
-  -${notification}
+${notification.body}
 </#list>
+</#if>
 
 View trip: ${OTP_UI_URL}${TRIPS_PATH}/${tripId}
 

src/main/resources/templates/MonitoredTripText.ftl

@@ -4,8 +4,14 @@
 -->
 
 Your trip has the following notifications:
-<#list notifications as notification>
-  -${notification}
-</#list>
+<#if notifications?size gt 1>
+  <#list notifications as notification>
+    • ${notification.body}
+  </#list>
+<#else>
+  <#list notifications as notification>
+    ${notification.body}
+  </#list>
+</#if>
 
 View trip: ${OTP_UI_URL}${TRIPS_PATH}/${tripId}

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.
+ * <p>
+ * 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<TemplateRenderingTestCase> createTemplateRenderingTestCases()
         // Trip Monitor Notifications tests (for CheckMonitoredTrip#sendNotifications).
         Map<String, Object> 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"
         ));

src/test/resources/snapshots/org/opentripplanner/middleware/utils/TemplateUtilsTest/Monitored_Trip_HTML_Email-0.json

@@ -1 +1 @@
-"<style>\n    body { \n        padding: 1em;\n    }\n\n    h1 {\n        font-size: 18px;\n        font-weight: 600;\n    }\n\n    small {\n        color: #515151\n    }\n</style>\n\n\n\n\n<body>\n    <div>\n<div>\n    <div>\n        <h1>Your trip has the following notifications:</h1>\n        <ul>\n          <li>Test notification.</li>\n          <li>Another notification.</li>\n        </ul>\n        <p>View all of your saved trips in <a href=\"https://plan.example.com/#/account/trips/18f642d5-f7a8-475a-9469-800129e6c0b3\">Trip Planner</a>.</p>\n    </div>\n</div>\n<hr/>\n<div>\n    <p>\n        <small>\n            You're receiving this email because you're subscribed to notifications through Trip Planner. You can manage that subscription <a href=\"https://plan.example.com/#/account\">here</a>.\n        </small>\n    </p>\n</div>\n    </div>\n</body>\n"
+"<style>\n    body { \n        padding: 1em;\n    }\n\n    h1 {\n        font-size: 18px;\n        font-weight: 600;\n    }\n\n    small {\n        color: #515151\n    }\n</style>\n\n\n\n\n<body>\n    <div>\n<div>\n    <div>\n        <h1>Your trip has the following notifications:</h1>\n\n                <p>This is the initial reminder text</p>\n\n        <ul>\n                    <li>This is the departure delay text</li>\n        </ul>\n\n        <p>View all of your saved trips in <a href=\"https://plan.example.com/#/account/trips/18f642d5-f7a8-475a-9469-800129e6c0b3\">Trip Planner</a>.</p>\n    </div>\n</div>\n<hr/>\n<div>\n    <p>\n        <small>\n            You're receiving this email because you're subscribed to notifications through Trip Planner. You can manage that subscription <a href=\"https://plan.example.com/#/account\">here</a>.\n        </small>\n    </p>\n</div>\n    </div>\n</body>\n"

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."
+"\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."

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"
+"\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"