From 9fc6822bc1363b1dd9e190e0806de04c41422bb6 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 4 Oct 2024 10:34:31 -0400 Subject: [PATCH 01/39] feat(TrackedJourney): Add method to compute total deviation. --- .../middleware/models/TrackedJourney.java | 12 ++++++++ .../middleware/models/TrackedJourneyTest.java | 28 +++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 src/test/java/org/opentripplanner/middleware/models/TrackedJourneyTest.java diff --git a/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java b/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java index a6b4ed5eb..c060e9c27 100644 --- a/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java +++ b/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java @@ -26,6 +26,8 @@ public class TrackedJourney extends Model { public Map busNotificationMessages = new HashMap<>(); + public Double totalDeviation; + public static final String TRIP_ID_FIELD_NAME = "tripId"; public static final String LOCATIONS_FIELD_NAME = "locations"; @@ -91,4 +93,14 @@ public void updateNotificationMessage(String routeId, String body) { busNotificationMessages ); } + + /** The sum of the deviations for all tracking locations that have it. */ + public double computeTotalDeviation() { + if (locations == null) return -1; + + return locations.stream() + .filter(l -> l.deviationMeters != null) + .map(l -> l.deviationMeters) + .reduce(0.0, Double::sum); + } } diff --git a/src/test/java/org/opentripplanner/middleware/models/TrackedJourneyTest.java b/src/test/java/org/opentripplanner/middleware/models/TrackedJourneyTest.java new file mode 100644 index 000000000..b99b75a57 --- /dev/null +++ b/src/test/java/org/opentripplanner/middleware/models/TrackedJourneyTest.java @@ -0,0 +1,28 @@ +package org.opentripplanner.middleware.models; + +import org.junit.jupiter.api.Test; +import org.opentripplanner.middleware.triptracker.TrackingLocation; + +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class TrackedJourneyTest { + @Test + void canComputeTotalDeviation() { + TrackedJourney journey = new TrackedJourney(); + journey.locations = null; + assertEquals(-1.0, journey.computeTotalDeviation()); + + journey.locations = Stream + .of(11.0, 23.0, 6.4) + .map(d -> { + TrackingLocation location = new TrackingLocation(); + location.deviationMeters = d; + return location; + }) + .collect(Collectors.toList()); + assertEquals(40.4, journey.computeTotalDeviation()); + } +} From 6507d5a0a24d84c25aafd0b486fb736fa315c16f Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 4 Oct 2024 12:04:21 -0400 Subject: [PATCH 02/39] feat(TrackedJourney): Persist total deviation upon completing journey. --- .../middleware/models/TrackedJourney.java | 2 ++ .../middleware/triptracker/ManageTripTracking.java | 2 ++ .../controllers/api/TrackedTripControllerTest.java | 13 +++++++++++++ 3 files changed, 17 insertions(+) diff --git a/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java b/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java index c060e9c27..1ce2462d9 100644 --- a/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java +++ b/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java @@ -37,6 +37,8 @@ public class TrackedJourney extends Model { public static final String END_CONDITION_FIELD_NAME = "endCondition"; + public static final String TOTAL_DEVIATION_FIELD_NAME = "totalDeviation"; + public static final String TERMINATED_BY_USER = "Tracking terminated by user."; public static final String FORCIBLY_TERMINATED = "Tracking forcibly terminated."; diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java b/src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java index 1332bdf12..1fd4daff6 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java @@ -152,6 +152,8 @@ private static EndTrackingResponse completeJourney(TripTrackingData tripData, bo trackedJourney.end(isForciblyEnded); Persistence.trackedJourneys.updateField(trackedJourney.id, TrackedJourney.END_TIME_FIELD_NAME, trackedJourney.endTime); Persistence.trackedJourneys.updateField(trackedJourney.id, TrackedJourney.END_CONDITION_FIELD_NAME, trackedJourney.endCondition); + trackedJourney.totalDeviation = trackedJourney.computeTotalDeviation(); + Persistence.trackedJourneys.updateField(trackedJourney.id, TrackedJourney.TOTAL_DEVIATION_FIELD_NAME, trackedJourney.totalDeviation); // Provide response. return new EndTrackingResponse( diff --git a/src/test/java/org/opentripplanner/middleware/controllers/api/TrackedTripControllerTest.java b/src/test/java/org/opentripplanner/middleware/controllers/api/TrackedTripControllerTest.java index 98a281447..70f3b1a5a 100644 --- a/src/test/java/org/opentripplanner/middleware/controllers/api/TrackedTripControllerTest.java +++ b/src/test/java/org/opentripplanner/middleware/controllers/api/TrackedTripControllerTest.java @@ -50,6 +50,8 @@ import static org.junit.jupiter.api.Assumptions.assumeTrue; import static org.opentripplanner.middleware.auth.Auth0Connection.restoreDefaultAuthDisabled; import static org.opentripplanner.middleware.auth.Auth0Connection.setAuthDisabled; +import static org.opentripplanner.middleware.models.TrackedJourney.FORCIBLY_TERMINATED; +import static org.opentripplanner.middleware.models.TrackedJourney.TERMINATED_BY_USER; import static org.opentripplanner.middleware.testutils.ApiTestUtils.TEMP_AUTH0_USER_PASSWORD; import static org.opentripplanner.middleware.testutils.ApiTestUtils.getMockHeaders; import static org.opentripplanner.middleware.testutils.ApiTestUtils.makeRequest; @@ -169,6 +171,11 @@ void canCompleteJourneyLifeCycle() throws Exception { assertEquals(TripStatus.ENDED.name(), endTrackingResponse.tripStatus); assertEquals(HttpStatus.OK_200, response.status); + // Check that the TrackedJourney Mongo record has been updated. + TrackedJourney mongoTrackedJourney = Persistence.trackedJourneys.getById(startTrackingResponse.journeyId); + assertEquals(TERMINATED_BY_USER, mongoTrackedJourney.endCondition); + assertNotNull(mongoTrackedJourney.totalDeviation); + assertNotEquals(0.0, mongoTrackedJourney.totalDeviation); DateTimeUtils.useSystemDefaultClockAndTimezone(); } @@ -316,6 +323,12 @@ void canForciblyEndJourney() throws Exception { var endTrackingResponse = JsonUtils.getPOJOFromJSON(response.responseBody, EndTrackingResponse.class); assertEquals(TripStatus.ENDED.name(), endTrackingResponse.tripStatus); assertEquals(HttpStatus.OK_200, response.status); + + // Check that the TrackedJourney Mongo record has been updated. + TrackedJourney mongoTrackedJourney = Persistence.trackedJourneys.getById(startTrackingResponse.journeyId); + assertEquals(FORCIBLY_TERMINATED, mongoTrackedJourney.endCondition); + assertNotNull(mongoTrackedJourney.totalDeviation); + assertNotEquals(0.0, mongoTrackedJourney.totalDeviation); } @Test From 44c456eb050be074064cd4af277e43bfef32e15f Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 8 Oct 2024 16:24:34 -0400 Subject: [PATCH 03/39] .feat(OtpUser): Add field for last survey notif sent. --- .../java/org/opentripplanner/middleware/models/OtpUser.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java index 495d35954..1dbc1d476 100644 --- a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java @@ -26,6 +26,7 @@ public enum Notification { public static final String AUTH0_SCOPE = "otp-user"; private static final long serialVersionUID = 1L; private static final Logger LOG = LoggerFactory.getLogger(OtpUser.class); + public static final String LAST_TRIP_SURVEY_NOTIF_SENT_FIELD = "lastTripSurveyNotificationSent"; /** Whether the user would like accessible routes by default. */ public boolean accessibilityRoutingByDefault; @@ -76,6 +77,9 @@ public enum Notification { /** Whether to store the user's trip history (user must opt in). */ public boolean storeTripHistory; + /** When the last post-trip survey notification was sent. */ + public Date lastTripSurveyNotificationSent; + @JsonIgnore /** If this user was created by an {@link ApiUser}, this parameter will match the {@link ApiUser}'s id */ public String applicationId; From 9e19b1ec8ce3d23e1a245ac41f82270de628d232 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 8 Oct 2024 16:25:53 -0400 Subject: [PATCH 04/39] refactor(ApiController): Extract const for mongo id field. --- .../middleware/controllers/api/ApiController.java | 3 ++- .../middleware/tripmonitor/jobs/MonitorAllTripsJob.java | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/controllers/api/ApiController.java b/src/main/java/org/opentripplanner/middleware/controllers/api/ApiController.java index d6b7ba2cc..16b2b20de 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/ApiController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/ApiController.java @@ -56,6 +56,7 @@ public abstract class ApiController implements Endpoint { public static final int DEFAULT_OFFSET = 0; public static final String OFFSET_PARAM = "offset"; public static final String USER_ID_PARAM = "userId"; + public static final String ID_FIELD_NAME = "_id"; public static final ParameterDescriptor LIMIT = ParameterDescriptor.newBuilder() .withName(LIMIT_PARAM) @@ -219,7 +220,7 @@ private ResponseList getMany(Request req, Response res) { // will be limited to just the entity matching this Otp user. Bson filter = (requestingUser.apiUser != null) ? Filters.eq("applicationId", requestingUser.apiUser.id) - : Filters.eq("_id", requestingUser.otpUser.id); + : Filters.eq(ID_FIELD_NAME, requestingUser.otpUser.id); return persistence.getResponseList(filter, offset, limit); } else if (requestingUser.isAPIUser()) { // A user id must be provided if the request is being made by a third party user. diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/MonitorAllTripsJob.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/MonitorAllTripsJob.java index 636c904b7..07f8af4af 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/MonitorAllTripsJob.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/MonitorAllTripsJob.java @@ -12,6 +12,8 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import static org.opentripplanner.middleware.controllers.api.ApiController.ID_FIELD_NAME; + /** * This job will analyze applicable monitored trips and create further individual tasks to analyze each individual trip. */ @@ -55,7 +57,7 @@ public void run() { // This saves bandwidth and memory, as only the ID field is used to set up this job. // The full data for each trip will be fetched at the time the actual analysis takes place. List allTripIds = Persistence.monitoredTrips.getDistinctFieldValues( - "_id", + ID_FIELD_NAME, makeTripFilter(), String.class ).into(new ArrayList<>()); From 462639bb2adc4cbab638eca87dbd482a9814f3a3 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 8 Oct 2024 16:31:12 -0400 Subject: [PATCH 05/39] refactor(MonitoredTrip): Extract const for trip id field. --- .../middleware/controllers/api/MonitoredTripController.java | 3 ++- .../org/opentripplanner/middleware/models/MonitoredTrip.java | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java b/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java index d6ca475ec..f20ac4f7d 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java @@ -22,6 +22,7 @@ import static io.github.manusant.ss.descriptor.MethodDescriptor.path; import static com.mongodb.client.model.Filters.eq; +import static org.opentripplanner.middleware.models.MonitoredTrip.USER_ID_FIELD_NAME; import static org.opentripplanner.middleware.utils.ConfigUtils.getConfigPropertyAsInt; import static org.opentripplanner.middleware.utils.HttpUtils.JSON_ONLY; import static org.opentripplanner.middleware.utils.JsonUtils.getPOJOFromRequestBody; @@ -216,7 +217,7 @@ private static ItineraryExistence checkItinerary(Request request, Response respo */ private void verifyBelowMaxNumTrips(String userId, Request request) { // filter monitored trip on user id to find out how many have already been saved - Bson filter = Filters.and(eq("userId", userId)); + Bson filter = Filters.and(eq(USER_ID_FIELD_NAME, userId)); long count = this.persistence.getCountFiltered(filter); if (count >= MAXIMUM_PERMITTED_MONITORED_TRIPS) { logMessageAndHalt( diff --git a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java index 290c90c8b..94f36cdc6 100644 --- a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java @@ -47,6 +47,8 @@ @JsonIgnoreProperties(ignoreUnknown = true) public class MonitoredTrip extends Model { + public static final String USER_ID_FIELD_NAME = "userId"; + /** * Mongo Id of the {@link OtpUser} who owns this monitored trip. */ From 1f318421928c4da7aafb5c168ebeb2c052bfe4ae Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 8 Oct 2024 16:31:53 -0400 Subject: [PATCH 06/39] feat(TripSurveySenderJob): Add basic logic for trip survey job. --- .../triptracker/TripSurveySenderJob.java | 128 +++++++++++++ .../triptracker/TripSurveySenderJobTest.java | 174 ++++++++++++++++++ 2 files changed, 302 insertions(+) create mode 100644 src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java create mode 100644 src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java new file mode 100644 index 000000000..a5759614f --- /dev/null +++ b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java @@ -0,0 +1,128 @@ +package org.opentripplanner.middleware.triptracker; + +import com.mongodb.client.model.Filters; +import org.bson.conversions.Bson; +import org.opentripplanner.middleware.models.MonitoredTrip; +import org.opentripplanner.middleware.models.OtpUser; +import org.opentripplanner.middleware.models.TrackedJourney; +import org.opentripplanner.middleware.persistence.Persistence; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.ArrayList; +import java.util.Comparator; +import java.util.Date; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.function.Function; +import java.util.stream.Collectors; + +import static org.opentripplanner.middleware.controllers.api.ApiController.ID_FIELD_NAME; +import static org.opentripplanner.middleware.models.MonitoredTrip.USER_ID_FIELD_NAME; +import static org.opentripplanner.middleware.models.OtpUser.LAST_TRIP_SURVEY_NOTIF_SENT_FIELD; +import static org.opentripplanner.middleware.models.TrackedJourney.END_CONDITION_FIELD_NAME; +import static org.opentripplanner.middleware.models.TrackedJourney.END_TIME_FIELD_NAME; +import static org.opentripplanner.middleware.models.TrackedJourney.FORCIBLY_TERMINATED; +import static org.opentripplanner.middleware.models.TrackedJourney.TERMINATED_BY_USER; + +/** + * This job will analyze completed trips with deviations and send survey notifications about select trips. + */ +public class TripSurveySenderJob implements Runnable { + private static final Logger LOG = LoggerFactory.getLogger(TripSurveySenderJob.class); + + @Override + public void run() { + long start = System.currentTimeMillis(); + LOG.info("TripSurveySenderJob started"); + + // Pick users for which the last survey notification was sent more than a week ago. + List usersWithNotificationsOverAWeekAgo = getUsersWithNotificationsOverAWeekAgo(); + + // Collect journeys that were completed/terminated in the past 24-48 hrs. (skip ongoing journeys). + List journeysCompletedInPast24To48Hours = getCompletedJourneysInPast24To48Hours(); + + // Map users to journeys + Map> usersToJourneys = mapJourneysToUsers(journeysCompletedInPast24To48Hours, usersWithNotificationsOverAWeekAgo); + + for (Map.Entry> entry : usersToJourneys.entrySet()) { + // Find journey with the largest total deviation. + Optional optJourney = selectMostDeviatedJourney(entry.getValue()); + if (optJourney.isPresent()) { + // Send push notification about that journey. + + // Store time of last sent survey notification for user. (new Mongo collection) + } + } + + LOG.info("TripSurveySenderJob completed in {} sec", (System.currentTimeMillis() - start) / 1000); + } + + /** + * Get users whose last trip survey notification was at least a week ago. + */ + public List getUsersWithNotificationsOverAWeekAgo() { + Date aWeekAgo = Date.from(Instant.now().minus(7, ChronoUnit.DAYS)); + Bson dateFilter = Filters.lte(LAST_TRIP_SURVEY_NOTIF_SENT_FIELD, aWeekAgo); + + return Persistence.otpUsers.getFiltered(dateFilter).into(new ArrayList<>()); + } + + /** + * Gets tracked journeys for all users that were completed in the past 24 hours. + */ + public List getCompletedJourneysInPast24To48Hours() { + Date twentyFourHoursAgo = Date.from(Instant.now().minus(24, ChronoUnit.HOURS)); + Date fortyEightHoursAgo = Date.from(Instant.now().minus(48, ChronoUnit.HOURS)); + Bson dateFilter = Filters.and( + Filters.gte(END_TIME_FIELD_NAME, fortyEightHoursAgo), + Filters.lte(END_TIME_FIELD_NAME, twentyFourHoursAgo) + ); + Bson completeFilter = Filters.eq(END_CONDITION_FIELD_NAME, TERMINATED_BY_USER); + Bson terminatedFilter = Filters.eq(END_CONDITION_FIELD_NAME, FORCIBLY_TERMINATED); + Bson overallFilter = Filters.and(dateFilter, Filters.or(completeFilter, terminatedFilter)); + + return Persistence.trackedJourneys.getFiltered(overallFilter).into(new ArrayList<>()); + } + + /** + * Gets the trips for the given journeys and users. + */ + public List getTripsForJourneysAndUsers(List journeys, List otpUsers) { + Set tripIds = journeys.stream().map(j -> j.tripId).collect(Collectors.toSet()); + Set userIds = otpUsers.stream().map(u -> u.id).collect(Collectors.toSet()); + + Bson tripIdFilter = Filters.in(ID_FIELD_NAME, tripIds); + Bson userIdFilter = Filters.in(USER_ID_FIELD_NAME, userIds); + Bson overallFilter = Filters.and(tripIdFilter, userIdFilter); + + return Persistence.monitoredTrips.getFiltered(overallFilter).into(new ArrayList<>()); + } + + /** + * Map journeys to users. + */ + public Map> mapJourneysToUsers(List journeys, List otpUsers) { + List trips = getTripsForJourneysAndUsers(journeys, otpUsers); + + Map userMap = otpUsers.stream().collect(Collectors.toMap(u -> u.id, Function.identity())); + + HashMap> map = new HashMap<>(); + for (MonitoredTrip trip : trips) { + List journeyList = map.computeIfAbsent(userMap.get(trip.userId), u -> new ArrayList<>()); + journeyList.addAll(journeys.stream().filter(j -> trip.id.equals(j.tripId)).collect(Collectors.toList())); + } + + return map; + } + + public Optional selectMostDeviatedJourney(List journeys) { + if (journeys == null) return Optional.empty(); + return journeys.stream().max(Comparator.comparingDouble(j -> j.totalDeviation)); + } +} diff --git a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java new file mode 100644 index 000000000..aca652d9f --- /dev/null +++ b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java @@ -0,0 +1,174 @@ +package org.opentripplanner.middleware.triptracker; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.opentripplanner.middleware.models.MonitoredTrip; +import org.opentripplanner.middleware.models.OtpUser; +import org.opentripplanner.middleware.models.TrackedJourney; +import org.opentripplanner.middleware.persistence.Persistence; +import org.opentripplanner.middleware.testutils.ApiTestUtils; +import org.opentripplanner.middleware.testutils.OtpMiddlewareTestEnvironment; +import org.opentripplanner.middleware.testutils.PersistenceTestUtils; + +import javax.sound.midi.Track; +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.Date; +import java.util.List; +import java.util.Map; +import java.util.Optional; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; +import static org.opentripplanner.middleware.models.TrackedJourney.FORCIBLY_TERMINATED; +import static org.opentripplanner.middleware.models.TrackedJourney.TERMINATED_BY_USER; + +class TripSurveySenderJobTest extends OtpMiddlewareTestEnvironment { + + private static OtpUser user1notifiedNow; + private static OtpUser user2notifiedAWeekAgo; + private static OtpUser user3neverNotified; + + private static List otpUsers = List.of(); + private static List journeys = List.of(); + private static MonitoredTrip trip; + + @BeforeAll + public static void setUp() { + assumeTrue(IS_END_TO_END); + + // Create users. and populate the date for last trip survey notification. + user1notifiedNow = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("test-user1")); + user2notifiedAWeekAgo = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("test-user2")); + user3neverNotified = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("test-user3")); + + Instant now = Instant.now(); + user1notifiedNow.lastTripSurveyNotificationSent = Date.from(now); + user2notifiedAWeekAgo.lastTripSurveyNotificationSent = Date.from(now.minus(8, ChronoUnit.DAYS)); + user3neverNotified.lastTripSurveyNotificationSent = null; + + otpUsers = List.of(user1notifiedNow, user2notifiedAWeekAgo, user3neverNotified); + otpUsers.forEach(user -> Persistence.otpUsers.replace(user.id, user)); + + // Use one user for all trips and journeys (trips will be deleted with OtpUser.delete() on tear down. + trip = new MonitoredTrip(); + trip.id = String.format("%s-trip-id", user1notifiedNow.id); + trip.userId = user1notifiedNow.id; + Persistence.monitoredTrips.create(trip); + } + + @AfterAll + public static void tearDown() { + assumeTrue(IS_END_TO_END); + + // Delete users + otpUsers.forEach(user -> { + OtpUser storedUser = Persistence.otpUsers.getById(user.id); + if (storedUser != null) storedUser.delete(false); + }); + } + + @AfterEach + void afterEach() { + assumeTrue(IS_END_TO_END); + + // Delete journeys + for (TrackedJourney journey : journeys) { + TrackedJourney storedJourney = Persistence.trackedJourneys.getById(journey.id); + if (storedJourney != null) storedJourney.delete(); + } + } + + @Test + void canGetUsersWithNotificationsOverAWeekAgo() { + assumeTrue(IS_END_TO_END); + + TripSurveySenderJob job = new TripSurveySenderJob(); + List usersWithNotificationsOverAWeekAgo = job.getUsersWithNotificationsOverAWeekAgo(); + + assertEquals(1, usersWithNotificationsOverAWeekAgo.size()); + assertEquals(user2notifiedAWeekAgo.id, usersWithNotificationsOverAWeekAgo.get(0).id); + } + + @Test + void canGetCompletedJourneysInPast24To48Hours() { + assumeTrue(IS_END_TO_END); + + Instant threeHoursAgo = Instant.now().minus(3, ChronoUnit.HOURS); + Instant thirtyHoursAgo = Instant.now().minus(30, ChronoUnit.HOURS); + Instant threeDaysAgo = Instant.now().minus(3, ChronoUnit.DAYS); + + // Create journey for each trip for all users above (they will be deleted explicitly after this test). + journeys = List.of( + // Ongoing journey (should not be included) + createJourney("ongoing-journey", trip.id, null, null), + + // Journey completed by user 30 hours ago (should be included) + createJourney("user-terminated-journey", trip.id, thirtyHoursAgo, TERMINATED_BY_USER), + + // Journey terminated forcibly 30 hours ago (should be included) + createJourney("forcibly-terminated-journey", trip.id, thirtyHoursAgo, FORCIBLY_TERMINATED), + + // Additional journey completed over 48 hours (should not be included). + createJourney("journey-done-3-days-ago", trip.id, threeDaysAgo, TERMINATED_BY_USER), + + // Additional journey completed within 24 hours (should not be included). + createJourney("journey-done-3-hours-ago", trip.id, threeHoursAgo, TERMINATED_BY_USER) + ); + + TripSurveySenderJob job = new TripSurveySenderJob(); + List completedJourneys = job.getCompletedJourneysInPast24To48Hours(); + + assertEquals(2, completedJourneys.size()); + } + + private static TrackedJourney createJourney(String id, String tripId, Instant endTime, String endCondition) { + TrackedJourney journey = new TrackedJourney(); + journey.id = id; + journey.tripId = tripId; + journey.endCondition = endCondition; + journey.endTime = endTime != null ? Date.from(endTime) : null; + Persistence.trackedJourneys.create(journey); + return journey; + } + + @Test + void canMapJourneysToUsers() { + // Create journey, some for the stored trip, others orphan (they will be deleted explicitly after this test). + journeys = List.of( + createJourney("journey-1", trip.id, null, null), + createJourney("journey-2", trip.id, null, null), + createJourney("journey-3", "other-trip", null, null), + createJourney("journey-4", "other-trip", null, null) + ); + + TripSurveySenderJob job = new TripSurveySenderJob(); + List trips = job.getTripsForJourneysAndUsers(journeys, otpUsers); + + assertEquals(1, trips.size()); + assertEquals(trip.id, trips.get(0).id); + + Map> usersToJourneys = job.mapJourneysToUsers(journeys, otpUsers); + assertEquals(1, usersToJourneys.size()); + assertEquals(List.of(journeys.get(0), journeys.get(1)), usersToJourneys.get(otpUsers.get(0))); + } + + @Test + void canSelectMostDeviatedJourney() { + TrackedJourney journey1 = new TrackedJourney(); + journey1.totalDeviation = 250.0; + journey1.endTime = Date.from(Instant.now().minus(3, ChronoUnit.HOURS)); + + TrackedJourney journey2 = new TrackedJourney(); + journey2.totalDeviation = 400.0; + journey2.endTime = Date.from(Instant.now().minus(5, ChronoUnit.HOURS)); + + TripSurveySenderJob job = new TripSurveySenderJob(); + Optional optJourney = job.selectMostDeviatedJourney(List.of(journey1, journey2)); + assertTrue(optJourney.isPresent()); + assertEquals(journey2, optJourney.get()); + } +} From 2e2473ff72f60e60d101ff36971a164b76184bd3 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 8 Oct 2024 16:47:36 -0400 Subject: [PATCH 07/39] refactor(TripSurveySenderJob): Convert methods to static. --- .../triptracker/TripSurveySenderJob.java | 10 +++++----- .../triptracker/TripSurveySenderJobTest.java | 20 +++++++------------ 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java index a5759614f..70d216f9d 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java @@ -66,7 +66,7 @@ public void run() { /** * Get users whose last trip survey notification was at least a week ago. */ - public List getUsersWithNotificationsOverAWeekAgo() { + public static List getUsersWithNotificationsOverAWeekAgo() { Date aWeekAgo = Date.from(Instant.now().minus(7, ChronoUnit.DAYS)); Bson dateFilter = Filters.lte(LAST_TRIP_SURVEY_NOTIF_SENT_FIELD, aWeekAgo); @@ -76,7 +76,7 @@ public List getUsersWithNotificationsOverAWeekAgo() { /** * Gets tracked journeys for all users that were completed in the past 24 hours. */ - public List getCompletedJourneysInPast24To48Hours() { + public static List getCompletedJourneysInPast24To48Hours() { Date twentyFourHoursAgo = Date.from(Instant.now().minus(24, ChronoUnit.HOURS)); Date fortyEightHoursAgo = Date.from(Instant.now().minus(48, ChronoUnit.HOURS)); Bson dateFilter = Filters.and( @@ -93,7 +93,7 @@ public List getCompletedJourneysInPast24To48Hours() { /** * Gets the trips for the given journeys and users. */ - public List getTripsForJourneysAndUsers(List journeys, List otpUsers) { + public static List getTripsForJourneysAndUsers(List journeys, List otpUsers) { Set tripIds = journeys.stream().map(j -> j.tripId).collect(Collectors.toSet()); Set userIds = otpUsers.stream().map(u -> u.id).collect(Collectors.toSet()); @@ -107,7 +107,7 @@ public List getTripsForJourneysAndUsers(List jour /** * Map journeys to users. */ - public Map> mapJourneysToUsers(List journeys, List otpUsers) { + public static Map> mapJourneysToUsers(List journeys, List otpUsers) { List trips = getTripsForJourneysAndUsers(journeys, otpUsers); Map userMap = otpUsers.stream().collect(Collectors.toMap(u -> u.id, Function.identity())); @@ -121,7 +121,7 @@ public Map> mapJourneysToUsers(List selectMostDeviatedJourney(List journeys) { + public static Optional selectMostDeviatedJourney(List journeys) { if (journeys == null) return Optional.empty(); return journeys.stream().max(Comparator.comparingDouble(j -> j.totalDeviation)); } diff --git a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java index aca652d9f..907f6bc91 100644 --- a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java +++ b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java @@ -12,7 +12,6 @@ import org.opentripplanner.middleware.testutils.OtpMiddlewareTestEnvironment; import org.opentripplanner.middleware.testutils.PersistenceTestUtils; -import javax.sound.midi.Track; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Date; @@ -86,9 +85,7 @@ void afterEach() { void canGetUsersWithNotificationsOverAWeekAgo() { assumeTrue(IS_END_TO_END); - TripSurveySenderJob job = new TripSurveySenderJob(); - List usersWithNotificationsOverAWeekAgo = job.getUsersWithNotificationsOverAWeekAgo(); - + List usersWithNotificationsOverAWeekAgo = TripSurveySenderJob.getUsersWithNotificationsOverAWeekAgo(); assertEquals(1, usersWithNotificationsOverAWeekAgo.size()); assertEquals(user2notifiedAWeekAgo.id, usersWithNotificationsOverAWeekAgo.get(0).id); } @@ -119,9 +116,7 @@ void canGetCompletedJourneysInPast24To48Hours() { createJourney("journey-done-3-hours-ago", trip.id, threeHoursAgo, TERMINATED_BY_USER) ); - TripSurveySenderJob job = new TripSurveySenderJob(); - List completedJourneys = job.getCompletedJourneysInPast24To48Hours(); - + List completedJourneys = TripSurveySenderJob.getCompletedJourneysInPast24To48Hours(); assertEquals(2, completedJourneys.size()); } @@ -137,6 +132,8 @@ private static TrackedJourney createJourney(String id, String tripId, Instant en @Test void canMapJourneysToUsers() { + assumeTrue(IS_END_TO_END); + // Create journey, some for the stored trip, others orphan (they will be deleted explicitly after this test). journeys = List.of( createJourney("journey-1", trip.id, null, null), @@ -145,13 +142,11 @@ void canMapJourneysToUsers() { createJourney("journey-4", "other-trip", null, null) ); - TripSurveySenderJob job = new TripSurveySenderJob(); - List trips = job.getTripsForJourneysAndUsers(journeys, otpUsers); - + List trips = TripSurveySenderJob.getTripsForJourneysAndUsers(journeys, otpUsers); assertEquals(1, trips.size()); assertEquals(trip.id, trips.get(0).id); - Map> usersToJourneys = job.mapJourneysToUsers(journeys, otpUsers); + Map> usersToJourneys = TripSurveySenderJob.mapJourneysToUsers(journeys, otpUsers); assertEquals(1, usersToJourneys.size()); assertEquals(List.of(journeys.get(0), journeys.get(1)), usersToJourneys.get(otpUsers.get(0))); } @@ -166,8 +161,7 @@ void canSelectMostDeviatedJourney() { journey2.totalDeviation = 400.0; journey2.endTime = Date.from(Instant.now().minus(5, ChronoUnit.HOURS)); - TripSurveySenderJob job = new TripSurveySenderJob(); - Optional optJourney = job.selectMostDeviatedJourney(List.of(journey1, journey2)); + Optional optJourney = TripSurveySenderJob.selectMostDeviatedJourney(List.of(journey1, journey2)); assertTrue(optJourney.isPresent()); assertEquals(journey2, optJourney.get()); } From 2432090a3ffcab2c48c51760b6a166726979865a Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 8 Oct 2024 17:27:53 -0400 Subject: [PATCH 08/39] test(TripSurveySenderJob): Reuse journeys. --- .../triptracker/TripSurveySenderJob.java | 3 +- .../triptracker/TripSurveySenderJobTest.java | 110 ++++++++++++------ 2 files changed, 76 insertions(+), 37 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java index 70d216f9d..dbbd14c45 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java @@ -56,7 +56,8 @@ public void run() { if (optJourney.isPresent()) { // Send push notification about that journey. - // Store time of last sent survey notification for user. (new Mongo collection) + // Store time of last sent survey notification for user. + Persistence.otpUsers.updateField(entry.getKey().id, LAST_TRIP_SURVEY_NOTIF_SENT_FIELD, new Date()); } } diff --git a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java index 907f6bc91..a2e9ee110 100644 --- a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java +++ b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java @@ -20,8 +20,11 @@ import java.util.Optional; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; +import static org.opentripplanner.middleware.models.OtpUser.LAST_TRIP_SURVEY_NOTIF_SENT_FIELD; import static org.opentripplanner.middleware.models.TrackedJourney.FORCIBLY_TERMINATED; import static org.opentripplanner.middleware.models.TrackedJourney.TERMINATED_BY_USER; @@ -34,6 +37,7 @@ class TripSurveySenderJobTest extends OtpMiddlewareTestEnvironment { private static List otpUsers = List.of(); private static List journeys = List.of(); private static MonitoredTrip trip; + private static final Date EIGHT_DAYS_AGO = Date.from(Instant.now().minus(8, ChronoUnit.DAYS)); @BeforeAll public static void setUp() { @@ -44,9 +48,8 @@ public static void setUp() { user2notifiedAWeekAgo = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("test-user2")); user3neverNotified = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("test-user3")); - Instant now = Instant.now(); - user1notifiedNow.lastTripSurveyNotificationSent = Date.from(now); - user2notifiedAWeekAgo.lastTripSurveyNotificationSent = Date.from(now.minus(8, ChronoUnit.DAYS)); + user1notifiedNow.lastTripSurveyNotificationSent = new Date(); + user2notifiedAWeekAgo.lastTripSurveyNotificationSent = EIGHT_DAYS_AGO; user3neverNotified.lastTripSurveyNotificationSent = null; otpUsers = List.of(user1notifiedNow, user2notifiedAWeekAgo, user3neverNotified); @@ -55,7 +58,7 @@ public static void setUp() { // Use one user for all trips and journeys (trips will be deleted with OtpUser.delete() on tear down. trip = new MonitoredTrip(); trip.id = String.format("%s-trip-id", user1notifiedNow.id); - trip.userId = user1notifiedNow.id; + trip.userId = user2notifiedAWeekAgo.id; Persistence.monitoredTrips.create(trip); } @@ -79,6 +82,9 @@ void afterEach() { TrackedJourney storedJourney = Persistence.trackedJourneys.getById(journey.id); if (storedJourney != null) storedJourney.delete(); } + + // Reset notification sent time to affected user. + Persistence.otpUsers.updateField(user2notifiedAWeekAgo.id, LAST_TRIP_SURVEY_NOTIF_SENT_FIELD, EIGHT_DAYS_AGO); } @Test @@ -93,39 +99,25 @@ void canGetUsersWithNotificationsOverAWeekAgo() { @Test void canGetCompletedJourneysInPast24To48Hours() { assumeTrue(IS_END_TO_END); - - Instant threeHoursAgo = Instant.now().minus(3, ChronoUnit.HOURS); - Instant thirtyHoursAgo = Instant.now().minus(30, ChronoUnit.HOURS); - Instant threeDaysAgo = Instant.now().minus(3, ChronoUnit.DAYS); - - // Create journey for each trip for all users above (they will be deleted explicitly after this test). - journeys = List.of( - // Ongoing journey (should not be included) - createJourney("ongoing-journey", trip.id, null, null), - - // Journey completed by user 30 hours ago (should be included) - createJourney("user-terminated-journey", trip.id, thirtyHoursAgo, TERMINATED_BY_USER), - - // Journey terminated forcibly 30 hours ago (should be included) - createJourney("forcibly-terminated-journey", trip.id, thirtyHoursAgo, FORCIBLY_TERMINATED), - - // Additional journey completed over 48 hours (should not be included). - createJourney("journey-done-3-days-ago", trip.id, threeDaysAgo, TERMINATED_BY_USER), - - // Additional journey completed within 24 hours (should not be included). - createJourney("journey-done-3-hours-ago", trip.id, threeHoursAgo, TERMINATED_BY_USER) - ); + createTestJourneys(); List completedJourneys = TripSurveySenderJob.getCompletedJourneysInPast24To48Hours(); assertEquals(2, completedJourneys.size()); } - private static TrackedJourney createJourney(String id, String tripId, Instant endTime, String endCondition) { + private static TrackedJourney createJourney( + String id, + String tripId, + Instant endTime, + String endCondition, + Double deviation + ) { TrackedJourney journey = new TrackedJourney(); journey.id = id; journey.tripId = tripId; journey.endCondition = endCondition; journey.endTime = endTime != null ? Date.from(endTime) : null; + journey.totalDeviation = deviation; Persistence.trackedJourneys.create(journey); return journey; } @@ -133,14 +125,7 @@ private static TrackedJourney createJourney(String id, String tripId, Instant en @Test void canMapJourneysToUsers() { assumeTrue(IS_END_TO_END); - - // Create journey, some for the stored trip, others orphan (they will be deleted explicitly after this test). - journeys = List.of( - createJourney("journey-1", trip.id, null, null), - createJourney("journey-2", trip.id, null, null), - createJourney("journey-3", "other-trip", null, null), - createJourney("journey-4", "other-trip", null, null) - ); + createTestJourneys(); List trips = TripSurveySenderJob.getTripsForJourneysAndUsers(journeys, otpUsers); assertEquals(1, trips.size()); @@ -148,7 +133,9 @@ void canMapJourneysToUsers() { Map> usersToJourneys = TripSurveySenderJob.mapJourneysToUsers(journeys, otpUsers); assertEquals(1, usersToJourneys.size()); - assertEquals(List.of(journeys.get(0), journeys.get(1)), usersToJourneys.get(otpUsers.get(0))); + List userJourneys = usersToJourneys.get(otpUsers.get(1)); + assertEquals(5, userJourneys.size()); + assertTrue(userJourneys.containsAll(journeys.subList(0, 4))); } @Test @@ -165,4 +152,55 @@ void canSelectMostDeviatedJourney() { assertTrue(optJourney.isPresent()); assertEquals(journey2, optJourney.get()); } + + @Test + void canRunJob() { + assumeTrue(IS_END_TO_END); + createTestJourneys(); + + Date start = new Date(); + OtpUser storedUser = Persistence.otpUsers.getById(user2notifiedAWeekAgo.id); + assertFalse(start.before(storedUser.lastTripSurveyNotificationSent)); + + TripSurveySenderJob job = new TripSurveySenderJob(); + job.run(); + + storedUser = Persistence.otpUsers.getById(user2notifiedAWeekAgo.id); + assertTrue(start.before(storedUser.lastTripSurveyNotificationSent)); + + // Other user last notification should not have changed. + storedUser = Persistence.otpUsers.getById(user1notifiedNow.id); + assertFalse(start.before(storedUser.lastTripSurveyNotificationSent)); + storedUser = Persistence.otpUsers.getById(user3neverNotified.id); + assertNull(storedUser.lastTripSurveyNotificationSent); + } + + private static void createTestJourneys() { + Instant threeHoursAgo = Instant.now().minus(3, ChronoUnit.HOURS); + Instant thirtyHoursAgo = Instant.now().minus(30, ChronoUnit.HOURS); + Instant threeDaysAgo = Instant.now().minus(3, ChronoUnit.DAYS); + + // Create journey for each trip for all users above (they will be deleted explicitly after each test). + journeys = List.of( + // Ongoing journey (should not be included) + createJourney("ongoing-journey", trip.id, null, null, 10.0), + + // Journey completed by user 30 hours ago (should be included) + createJourney("user-terminated-journey", trip.id, thirtyHoursAgo, TERMINATED_BY_USER, 200.0), + + // Journey terminated forcibly 30 hours ago (should be included) + createJourney("forcibly-terminated-journey", trip.id, thirtyHoursAgo, FORCIBLY_TERMINATED, 400.0), + + // Additional journey completed over 48 hours (should not be included). + createJourney("journey-done-3-days-ago", trip.id, threeDaysAgo, TERMINATED_BY_USER, 10.0), + + // Additional journey completed within 24 hours (should not be included). + createJourney("journey-done-3-hours-ago", trip.id, threeHoursAgo, TERMINATED_BY_USER, 10.0), + + // Orphan journeys (should not be included) + createJourney("journey-3", "other-trip", null, null, 10.0), + createJourney("journey-4", "other-trip", null, null, 0.0) + ); + } } + From c00ac92f79b2be2e8fd8b0ee004b7a1513983ae7 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 9 Oct 2024 11:40:07 -0400 Subject: [PATCH 09/39] feat(TripSurveySenderJob): Send push notification using current svc for trip surveys. --- .../middleware/models/TrackedJourney.java | 2 ++ .../triptracker/TripSurveySenderJob.java | 21 ++++++++++++++++--- .../resources/templates/MonitoredTripPush.ftl | 2 +- .../templates/PostTripSurveyPush.ftl | 9 ++++++++ .../triptracker/TripSurveySenderJobTest.java | 7 +++++++ 5 files changed, 37 insertions(+), 4 deletions(-) create mode 100644 src/main/resources/templates/PostTripSurveyPush.ftl diff --git a/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java b/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java index 1ce2462d9..f16040b2d 100644 --- a/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java +++ b/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java @@ -28,6 +28,8 @@ public class TrackedJourney extends Model { public Double totalDeviation; + public transient MonitoredTrip trip; + public static final String TRIP_ID_FIELD_NAME = "tripId"; public static final String LOCATIONS_FIELD_NAME = "locations"; diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java index dbbd14c45..243b7a14d 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java @@ -6,6 +6,9 @@ import org.opentripplanner.middleware.models.OtpUser; import org.opentripplanner.middleware.models.TrackedJourney; import org.opentripplanner.middleware.persistence.Persistence; +import org.opentripplanner.middleware.utils.DateTimeUtils; +import org.opentripplanner.middleware.utils.I18nUtils; +import org.opentripplanner.middleware.utils.NotificationUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -47,7 +50,7 @@ public void run() { // Collect journeys that were completed/terminated in the past 24-48 hrs. (skip ongoing journeys). List journeysCompletedInPast24To48Hours = getCompletedJourneysInPast24To48Hours(); - // Map users to journeys + // Map users to journeys. Map> usersToJourneys = mapJourneysToUsers(journeysCompletedInPast24To48Hours, usersWithNotificationsOverAWeekAgo); for (Map.Entry> entry : usersToJourneys.entrySet()) { @@ -55,9 +58,16 @@ public void run() { Optional optJourney = selectMostDeviatedJourney(entry.getValue()); if (optJourney.isPresent()) { // Send push notification about that journey. + OtpUser otpUser = entry.getKey(); + TrackedJourney journey = optJourney.get(); + MonitoredTrip trip = journey.trip; + Map data = new HashMap<>(); + data.put("tripDay", DateTimeUtils.makeOtpZonedDateTime(journey.startTime).getDayOfWeek()); + data.put("tripTime", DateTimeUtils.formatShortDate(trip.itinerary.startTime, I18nUtils.getOtpUserLocale(otpUser))); + NotificationUtils.sendPush(otpUser, "PostTripSurveyPush.ftl", data, trip.tripName, trip.id); // Store time of last sent survey notification for user. - Persistence.otpUsers.updateField(entry.getKey().id, LAST_TRIP_SURVEY_NOTIF_SENT_FIELD, new Date()); + Persistence.otpUsers.updateField(otpUser.id, LAST_TRIP_SURVEY_NOTIF_SENT_FIELD, new Date()); } } @@ -116,7 +126,12 @@ public static Map> mapJourneysToUsers(List> map = new HashMap<>(); for (MonitoredTrip trip : trips) { List journeyList = map.computeIfAbsent(userMap.get(trip.userId), u -> new ArrayList<>()); - journeyList.addAll(journeys.stream().filter(j -> trip.id.equals(j.tripId)).collect(Collectors.toList())); + for (TrackedJourney journey : journeys) { + if (trip.id.equals(journey.tripId)) { + journey.trip = trip; + journeyList.add(journey); + } + } } return map; diff --git a/src/main/resources/templates/MonitoredTripPush.ftl b/src/main/resources/templates/MonitoredTripPush.ftl index 68ce7b842..35ef5c6bc 100644 --- a/src/main/resources/templates/MonitoredTripPush.ftl +++ b/src/main/resources/templates/MonitoredTripPush.ftl @@ -3,7 +3,7 @@ OTP user's monitored trip. Note the following character limitations by mobile OS: - iOS: 178 characters over up to 4 lines, - - Android: 240 characters (We are not using notification title at this time). + - Android: 240 characters (excluding notification title). The max length is thus 178 characters. - List alerts with bullets if there are more than one of them. --> diff --git a/src/main/resources/templates/PostTripSurveyPush.ftl b/src/main/resources/templates/PostTripSurveyPush.ftl new file mode 100644 index 000000000..c40f0bf47 --- /dev/null +++ b/src/main/resources/templates/PostTripSurveyPush.ftl @@ -0,0 +1,9 @@ +<#-- + This is a template for push notifications content for notifications + after an OTP user takes a monitored trip and completes travel. + Note the following character limitations by mobile OS: + - iOS: 178 characters over up to 4 lines, + - Android: 240 characters (excluding notification title). + The max length is thus 178 characters. +--> +How was your trip on ${tripDay} at ${tripTime}? Tap for a quick survey. diff --git a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java index a2e9ee110..1c267cfea 100644 --- a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java +++ b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java @@ -7,6 +7,7 @@ import org.opentripplanner.middleware.models.MonitoredTrip; import org.opentripplanner.middleware.models.OtpUser; import org.opentripplanner.middleware.models.TrackedJourney; +import org.opentripplanner.middleware.otp.response.Itinerary; import org.opentripplanner.middleware.persistence.Persistence; import org.opentripplanner.middleware.testutils.ApiTestUtils; import org.opentripplanner.middleware.testutils.OtpMiddlewareTestEnvironment; @@ -59,6 +60,8 @@ public static void setUp() { trip = new MonitoredTrip(); trip.id = String.format("%s-trip-id", user1notifiedNow.id); trip.userId = user2notifiedAWeekAgo.id; + trip.itinerary = new Itinerary(); + trip.itinerary.startTime = new Date(); Persistence.monitoredTrips.create(trip); } @@ -117,6 +120,7 @@ private static TrackedJourney createJourney( journey.tripId = tripId; journey.endCondition = endCondition; journey.endTime = endTime != null ? Date.from(endTime) : null; + journey.startTime = journey.endTime; journey.totalDeviation = deviation; Persistence.trackedJourneys.create(journey); return journey; @@ -136,6 +140,9 @@ void canMapJourneysToUsers() { List userJourneys = usersToJourneys.get(otpUsers.get(1)); assertEquals(5, userJourneys.size()); assertTrue(userJourneys.containsAll(journeys.subList(0, 4))); + for (TrackedJourney journey : userJourneys) { + assertEquals(trip, journey.trip); + } } @Test From ca2b8aaa95d6d43aaae8dfef77cf0ec4ab91ad56 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 9 Oct 2024 11:47:28 -0400 Subject: [PATCH 10/39] feat(OtpMiddlewareMain): Schedule trip survey job. --- .../opentripplanner/middleware/OtpMiddlewareMain.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java b/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java index e8d09b7c5..8fc0056c7 100644 --- a/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java +++ b/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java @@ -23,6 +23,7 @@ import org.opentripplanner.middleware.otp.OtpVersion; import org.opentripplanner.middleware.persistence.Persistence; import org.opentripplanner.middleware.tripmonitor.jobs.MonitorAllTripsJob; +import org.opentripplanner.middleware.triptracker.TripSurveySenderJob; import org.opentripplanner.middleware.utils.ConfigUtils; import org.opentripplanner.middleware.utils.HttpUtils; import org.opentripplanner.middleware.utils.Scheduler; @@ -84,6 +85,16 @@ public static void main(String[] args) throws IOException, InterruptedException 1, TimeUnit.MINUTES ); + + // Schedule recurring job for post-trip surveys, once every few hours + // TODO: Determine whether this should go in some other process. + TripSurveySenderJob tripSurveySenderJob = new TripSurveySenderJob(); + Scheduler.scheduleJob( + tripSurveySenderJob, + 0, + 12, + TimeUnit.HOURS + ); } } From 20fdb33fe35ed193ae6ba18a508b6b0e14fc9ee9 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 9 Oct 2024 11:49:03 -0400 Subject: [PATCH 11/39] docs(swagger): Update snapshot. --- src/main/resources/latest-spark-swagger-output.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/resources/latest-spark-swagger-output.yaml b/src/main/resources/latest-spark-swagger-output.yaml index 89633d291..1b602da4f 100644 --- a/src/main/resources/latest-spark-swagger-output.yaml +++ b/src/main/resources/latest-spark-swagger-output.yaml @@ -2966,6 +2966,9 @@ definitions: $ref: "#/definitions/UserLocation" storeTripHistory: type: "boolean" + lastTripSurveyNotificationSent: + type: "string" + format: "date" applicationId: type: "string" MobilityProfile: From ab5a82f4f1bad46dfab1fa46b202866d7662fc8a Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 9 Oct 2024 13:10:39 -0400 Subject: [PATCH 12/39] fix(TripSurveySenderJob): Include missing last trip survey field in users to notify. --- .../middleware/triptracker/TripSurveySenderJob.java | 4 +++- .../middleware/triptracker/TripSurveySenderJobTest.java | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java index 243b7a14d..fcf6bb5ee 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java @@ -80,8 +80,10 @@ public void run() { public static List getUsersWithNotificationsOverAWeekAgo() { Date aWeekAgo = Date.from(Instant.now().minus(7, ChronoUnit.DAYS)); Bson dateFilter = Filters.lte(LAST_TRIP_SURVEY_NOTIF_SENT_FIELD, aWeekAgo); + Bson surveyNotSentFilter = Filters.not(Filters.exists(LAST_TRIP_SURVEY_NOTIF_SENT_FIELD)); + Bson overallFilter = Filters.or(dateFilter, surveyNotSentFilter); - return Persistence.otpUsers.getFiltered(dateFilter).into(new ArrayList<>()); + return Persistence.otpUsers.getFiltered(overallFilter).into(new ArrayList<>()); } /** diff --git a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java index 1c267cfea..1de38d15f 100644 --- a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java +++ b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java @@ -95,8 +95,10 @@ void canGetUsersWithNotificationsOverAWeekAgo() { assumeTrue(IS_END_TO_END); List usersWithNotificationsOverAWeekAgo = TripSurveySenderJob.getUsersWithNotificationsOverAWeekAgo(); - assertEquals(1, usersWithNotificationsOverAWeekAgo.size()); - assertEquals(user2notifiedAWeekAgo.id, usersWithNotificationsOverAWeekAgo.get(0).id); + assertEquals(2, usersWithNotificationsOverAWeekAgo.size()); + List expectedUserIds = List.of(user2notifiedAWeekAgo.id, user3neverNotified.id); + assertTrue(expectedUserIds.contains(usersWithNotificationsOverAWeekAgo.get(0).id)); + assertTrue(expectedUserIds.contains(usersWithNotificationsOverAWeekAgo.get(1).id)); } @Test From 828afdc599f8e9dfe959e7076b10b937b0aaba3d Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 9 Oct 2024 14:56:20 -0400 Subject: [PATCH 13/39] refactor(TripSurveySenderJob): Remove day of week in survey push message. --- .../middleware/triptracker/TripSurveySenderJob.java | 6 +++--- src/main/resources/templates/PostTripSurveyPush.ftl | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java index fcf6bb5ee..f07f4b6b0 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java @@ -61,9 +61,9 @@ public void run() { OtpUser otpUser = entry.getKey(); TrackedJourney journey = optJourney.get(); MonitoredTrip trip = journey.trip; - Map data = new HashMap<>(); - data.put("tripDay", DateTimeUtils.makeOtpZonedDateTime(journey.startTime).getDayOfWeek()); - data.put("tripTime", DateTimeUtils.formatShortDate(trip.itinerary.startTime, I18nUtils.getOtpUserLocale(otpUser))); + Map data = Map.of( + "tripTime", DateTimeUtils.formatShortDate(trip.itinerary.startTime, I18nUtils.getOtpUserLocale(otpUser)) + ); NotificationUtils.sendPush(otpUser, "PostTripSurveyPush.ftl", data, trip.tripName, trip.id); // Store time of last sent survey notification for user. diff --git a/src/main/resources/templates/PostTripSurveyPush.ftl b/src/main/resources/templates/PostTripSurveyPush.ftl index c40f0bf47..c6501ee2d 100644 --- a/src/main/resources/templates/PostTripSurveyPush.ftl +++ b/src/main/resources/templates/PostTripSurveyPush.ftl @@ -6,4 +6,4 @@ - Android: 240 characters (excluding notification title). The max length is thus 178 characters. --> -How was your trip on ${tripDay} at ${tripTime}? Tap for a quick survey. +How was your most recent trip starting at ${tripTime}? Tap for a quick survey. From 3ed298b6dcf45edaafdbe0683ee872f3814b9a8f Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 10 Oct 2024 14:25:56 -0400 Subject: [PATCH 14/39] docs(OtpMiddlewareMain): Fix trip survey comment typo. --- .../org/opentripplanner/middleware/OtpMiddlewareMain.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java b/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java index 8fc0056c7..c4c68906c 100644 --- a/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java +++ b/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java @@ -86,13 +86,13 @@ public static void main(String[] args) throws IOException, InterruptedException TimeUnit.MINUTES ); - // Schedule recurring job for post-trip surveys, once every few hours + // Schedule recurring job for post-trip surveys, once every hour to catch recently completed trips. // TODO: Determine whether this should go in some other process. TripSurveySenderJob tripSurveySenderJob = new TripSurveySenderJob(); Scheduler.scheduleJob( tripSurveySenderJob, 0, - 12, + 1, TimeUnit.HOURS ); } From fa6b8382c85a709789d2a1bd475124303501e14c Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 10 Oct 2024 17:48:43 -0400 Subject: [PATCH 15/39] refactor(TrackedJourney): Introduce consecutive deviation metric. --- .../middleware/models/TrackedJourney.java | 24 +++++++++++++ .../triptracker/TripSurveySenderJob.java | 7 +++- .../middleware/models/TrackedJourneyTest.java | 23 +++++++++++++ .../triptracker/TripSurveySenderJobTest.java | 34 ++++++++++++++----- 4 files changed, 78 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java b/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java index f16040b2d..f17d2bd70 100644 --- a/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java +++ b/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java @@ -3,6 +3,7 @@ import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import org.opentripplanner.middleware.persistence.Persistence; import org.opentripplanner.middleware.triptracker.TrackingLocation; +import org.opentripplanner.middleware.triptracker.TripStatus; import java.util.ArrayList; import java.util.Date; @@ -28,6 +29,8 @@ public class TrackedJourney extends Model { public Double totalDeviation; + public int longestConsecutiveDeviatedPoints; + public transient MonitoredTrip trip; public static final String TRIP_ID_FIELD_NAME = "tripId"; @@ -107,4 +110,25 @@ public double computeTotalDeviation() { .map(l -> l.deviationMeters) .reduce(0.0, Double::sum); } + + /** The largest consecutive deviations for all tracking locations marked "deviated". */ + public int computeLargestConsecutiveDeviations() { + if (locations == null) return -1; + + int count = 0; + int maxCount = 0; + for (TrackingLocation location : locations) { + // Traveler must be moving (speed != 0) for a deviated location to be counted. + if (location.tripStatus == TripStatus.DEVIATED) { + if (location.speed != 0) { + count++; + if (maxCount < count) maxCount = count; + } + } else { + // If a location is not deviated, reset the streak. + count = 0; + } + } + return maxCount; + } } diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java index f07f4b6b0..a8c62fd9b 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java @@ -55,7 +55,7 @@ public void run() { for (Map.Entry> entry : usersToJourneys.entrySet()) { // Find journey with the largest total deviation. - Optional optJourney = selectMostDeviatedJourney(entry.getValue()); + Optional optJourney = selectMostDeviatedJourneyUsingDeviatedPoints(entry.getValue()); if (optJourney.isPresent()) { // Send push notification about that journey. OtpUser otpUser = entry.getKey(); @@ -143,4 +143,9 @@ public static Optional selectMostDeviatedJourney(List j.totalDeviation)); } + + public static Optional selectMostDeviatedJourneyUsingDeviatedPoints(List journeys) { + if (journeys == null) return Optional.empty(); + return journeys.stream().max(Comparator.comparingInt(j -> j.longestConsecutiveDeviatedPoints)); + } } diff --git a/src/test/java/org/opentripplanner/middleware/models/TrackedJourneyTest.java b/src/test/java/org/opentripplanner/middleware/models/TrackedJourneyTest.java index b99b75a57..974000ccc 100644 --- a/src/test/java/org/opentripplanner/middleware/models/TrackedJourneyTest.java +++ b/src/test/java/org/opentripplanner/middleware/models/TrackedJourneyTest.java @@ -2,6 +2,7 @@ import org.junit.jupiter.api.Test; import org.opentripplanner.middleware.triptracker.TrackingLocation; +import org.opentripplanner.middleware.triptracker.TripStatus; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -25,4 +26,26 @@ void canComputeTotalDeviation() { .collect(Collectors.toList()); assertEquals(40.4, journey.computeTotalDeviation()); } + + @Test + void canComputeLargestConsecutiveDeviations() { + TrackedJourney journey = new TrackedJourney(); + journey.locations = null; + assertEquals(-1, journey.computeLargestConsecutiveDeviations()); + + journey.locations = Stream + .of(0, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 0) + .map(d -> { + TrackingLocation location = new TrackingLocation(); + location.tripStatus = d == 1 ? TripStatus.DEVIATED : TripStatus.ON_SCHEDULE; + location.speed = 1; + return location; + }) + .collect(Collectors.toList()); + + // Insert a location where the traveler is not moving. + journey.locations.get(4).speed = 0; + + assertEquals(4, journey.computeLargestConsecutiveDeviations()); + } } diff --git a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java index 1de38d15f..844725e5f 100644 --- a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java +++ b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java @@ -115,7 +115,7 @@ private static TrackedJourney createJourney( String tripId, Instant endTime, String endCondition, - Double deviation + int points ) { TrackedJourney journey = new TrackedJourney(); journey.id = id; @@ -123,7 +123,8 @@ private static TrackedJourney createJourney( journey.endCondition = endCondition; journey.endTime = endTime != null ? Date.from(endTime) : null; journey.startTime = journey.endTime; - journey.totalDeviation = deviation; + journey.totalDeviation = (double)points; + journey.longestConsecutiveDeviatedPoints = points; Persistence.trackedJourneys.create(journey); return journey; } @@ -162,6 +163,21 @@ void canSelectMostDeviatedJourney() { assertEquals(journey2, optJourney.get()); } + @Test + void canSelectMostDeviatedJourneyUsingDeviatedPoints() { + TrackedJourney journey1 = new TrackedJourney(); + journey1.longestConsecutiveDeviatedPoints = 250; + journey1.endTime = Date.from(Instant.now().minus(3, ChronoUnit.HOURS)); + + TrackedJourney journey2 = new TrackedJourney(); + journey2.longestConsecutiveDeviatedPoints = 400; + journey2.endTime = Date.from(Instant.now().minus(5, ChronoUnit.HOURS)); + + Optional optJourney = TripSurveySenderJob.selectMostDeviatedJourneyUsingDeviatedPoints(List.of(journey1, journey2)); + assertTrue(optJourney.isPresent()); + assertEquals(journey2, optJourney.get()); + } + @Test void canRunJob() { assumeTrue(IS_END_TO_END); @@ -192,23 +208,23 @@ private static void createTestJourneys() { // Create journey for each trip for all users above (they will be deleted explicitly after each test). journeys = List.of( // Ongoing journey (should not be included) - createJourney("ongoing-journey", trip.id, null, null, 10.0), + createJourney("ongoing-journey", trip.id, null, null, 10), // Journey completed by user 30 hours ago (should be included) - createJourney("user-terminated-journey", trip.id, thirtyHoursAgo, TERMINATED_BY_USER, 200.0), + createJourney("user-terminated-journey", trip.id, thirtyHoursAgo, TERMINATED_BY_USER, 200), // Journey terminated forcibly 30 hours ago (should be included) - createJourney("forcibly-terminated-journey", trip.id, thirtyHoursAgo, FORCIBLY_TERMINATED, 400.0), + createJourney("forcibly-terminated-journey", trip.id, thirtyHoursAgo, FORCIBLY_TERMINATED, 400), // Additional journey completed over 48 hours (should not be included). - createJourney("journey-done-3-days-ago", trip.id, threeDaysAgo, TERMINATED_BY_USER, 10.0), + createJourney("journey-done-3-days-ago", trip.id, threeDaysAgo, TERMINATED_BY_USER, 10), // Additional journey completed within 24 hours (should not be included). - createJourney("journey-done-3-hours-ago", trip.id, threeHoursAgo, TERMINATED_BY_USER, 10.0), + createJourney("journey-done-3-hours-ago", trip.id, threeHoursAgo, TERMINATED_BY_USER, 10), // Orphan journeys (should not be included) - createJourney("journey-3", "other-trip", null, null, 10.0), - createJourney("journey-4", "other-trip", null, null, 0.0) + createJourney("journey-3", "other-trip", null, null, 10), + createJourney("journey-4", "other-trip", null, null, 0) ); } } From 72ecf807943e46fd268a508d8aa2d2f6d1fa2ca8 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 10 Oct 2024 17:59:30 -0400 Subject: [PATCH 16/39] refactor(TripSurveySenderJob): Add a minimum 1-minute deviation threshold to select trips. --- .../triptracker/TripSurveySenderJob.java | 6 +++++- .../triptracker/TripSurveySenderJobTest.java | 14 ++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java index a8c62fd9b..927a93b12 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java @@ -32,6 +32,7 @@ import static org.opentripplanner.middleware.models.TrackedJourney.END_TIME_FIELD_NAME; import static org.opentripplanner.middleware.models.TrackedJourney.FORCIBLY_TERMINATED; import static org.opentripplanner.middleware.models.TrackedJourney.TERMINATED_BY_USER; +import static org.opentripplanner.middleware.triptracker.ManageTripTracking.TRIP_TRACKING_UPDATE_FREQUENCY_SECONDS; /** * This job will analyze completed trips with deviations and send survey notifications about select trips. @@ -146,6 +147,9 @@ public static Optional selectMostDeviatedJourney(List selectMostDeviatedJourneyUsingDeviatedPoints(List journeys) { if (journeys == null) return Optional.empty(); - return journeys.stream().max(Comparator.comparingInt(j -> j.longestConsecutiveDeviatedPoints)); + final double INTERVALS_IN_ONE_MINUTE = Math.ceil(60.0 / TRIP_TRACKING_UPDATE_FREQUENCY_SECONDS); + return journeys.stream() + .filter(j -> j.longestConsecutiveDeviatedPoints >= INTERVALS_IN_ONE_MINUTE) + .max(Comparator.comparingInt(j -> j.longestConsecutiveDeviatedPoints)); } } diff --git a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java index 844725e5f..a37a9929e 100644 --- a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java +++ b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java @@ -178,6 +178,20 @@ void canSelectMostDeviatedJourneyUsingDeviatedPoints() { assertEquals(journey2, optJourney.get()); } + @Test + void canSelectDeviatedJourneyWithDeviatedPointsThreshold() { + TrackedJourney journey1 = new TrackedJourney(); + journey1.longestConsecutiveDeviatedPoints = 6; + journey1.endTime = Date.from(Instant.now().minus(3, ChronoUnit.HOURS)); + + TrackedJourney journey2 = new TrackedJourney(); + journey2.longestConsecutiveDeviatedPoints = 8; + journey2.endTime = Date.from(Instant.now().minus(5, ChronoUnit.HOURS)); + + Optional optJourney = TripSurveySenderJob.selectMostDeviatedJourneyUsingDeviatedPoints(List.of(journey1, journey2)); + assertFalse(optJourney.isPresent()); + } + @Test void canRunJob() { assumeTrue(IS_END_TO_END); From be373351a958c3527a81e4233fc6bc360b1c4c57 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Mon, 21 Oct 2024 17:09:22 -0400 Subject: [PATCH 17/39] refactor(TrackedJourney): Remove cumulative deviation field. --- .../middleware/models/TrackedJourney.java | 27 ++++++++++-------- .../triptracker/ManageTripTracking.java | 4 +-- .../triptracker/TripSurveySenderJob.java | 5 ---- .../api/TrackedTripControllerTest.java | 6 ++-- .../middleware/models/TrackedJourneyTest.java | 28 ++++++------------- .../triptracker/TripSurveySenderJobTest.java | 16 ----------- 6 files changed, 27 insertions(+), 59 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java b/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java index f17d2bd70..3558940b4 100644 --- a/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java +++ b/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java @@ -27,9 +27,7 @@ public class TrackedJourney extends Model { public Map busNotificationMessages = new HashMap<>(); - public Double totalDeviation; - - public int longestConsecutiveDeviatedPoints; + public int longestConsecutiveDeviatedPoints = -1; public transient MonitoredTrip trip; @@ -42,7 +40,7 @@ public class TrackedJourney extends Model { public static final String END_CONDITION_FIELD_NAME = "endCondition"; - public static final String TOTAL_DEVIATION_FIELD_NAME = "totalDeviation"; + public static final String LONGEST_CONSECUTIVE_DEVIATED_POINTS_FIELD_NAME = "longestConsecutiveDeviatedPoints"; public static final String TERMINATED_BY_USER = "Tracking terminated by user."; @@ -118,15 +116,20 @@ public int computeLargestConsecutiveDeviations() { int count = 0; int maxCount = 0; for (TrackingLocation location : locations) { - // Traveler must be moving (speed != 0) for a deviated location to be counted. - if (location.tripStatus == TripStatus.DEVIATED) { - if (location.speed != 0) { - count++; - if (maxCount < count) maxCount = count; + // A trip status must have been computed for a location to count. + // (The mobile app will send many other more for reference, but only those for which we compute a status + // (i.e. the last coordinate in every batch) will potentially count. + if (location.tripStatus != null) { + // Traveler must be moving (speed != 0) for a deviated location to be counted. + if (location.tripStatus == TripStatus.DEVIATED) { + if (location.speed != 0) { + count++; + if (maxCount < count) maxCount = count; + } + } else { + // If a location has a status computed and is not deviated, reset the streak. + count = 0; } - } else { - // If a location is not deviated, reset the streak. - count = 0; } } return maxCount; diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java b/src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java index 1fd4daff6..9466d8527 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java @@ -152,8 +152,8 @@ private static EndTrackingResponse completeJourney(TripTrackingData tripData, bo trackedJourney.end(isForciblyEnded); Persistence.trackedJourneys.updateField(trackedJourney.id, TrackedJourney.END_TIME_FIELD_NAME, trackedJourney.endTime); Persistence.trackedJourneys.updateField(trackedJourney.id, TrackedJourney.END_CONDITION_FIELD_NAME, trackedJourney.endCondition); - trackedJourney.totalDeviation = trackedJourney.computeTotalDeviation(); - Persistence.trackedJourneys.updateField(trackedJourney.id, TrackedJourney.TOTAL_DEVIATION_FIELD_NAME, trackedJourney.totalDeviation); + trackedJourney.longestConsecutiveDeviatedPoints = trackedJourney.computeLargestConsecutiveDeviations(); + Persistence.trackedJourneys.updateField(trackedJourney.id, TrackedJourney.LONGEST_CONSECUTIVE_DEVIATED_POINTS_FIELD_NAME, trackedJourney.longestConsecutiveDeviatedPoints); // Provide response. return new EndTrackingResponse( diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java index 927a93b12..2638ce15c 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java @@ -140,11 +140,6 @@ public static Map> mapJourneysToUsers(List selectMostDeviatedJourney(List journeys) { - if (journeys == null) return Optional.empty(); - return journeys.stream().max(Comparator.comparingDouble(j -> j.totalDeviation)); - } - public static Optional selectMostDeviatedJourneyUsingDeviatedPoints(List journeys) { if (journeys == null) return Optional.empty(); final double INTERVALS_IN_ONE_MINUTE = Math.ceil(60.0 / TRIP_TRACKING_UPDATE_FREQUENCY_SECONDS); diff --git a/src/test/java/org/opentripplanner/middleware/controllers/api/TrackedTripControllerTest.java b/src/test/java/org/opentripplanner/middleware/controllers/api/TrackedTripControllerTest.java index 2e8db096e..89def54b4 100644 --- a/src/test/java/org/opentripplanner/middleware/controllers/api/TrackedTripControllerTest.java +++ b/src/test/java/org/opentripplanner/middleware/controllers/api/TrackedTripControllerTest.java @@ -190,8 +190,7 @@ void canCompleteJourneyLifeCycle() throws Exception { // Check that the TrackedJourney Mongo record has been updated. TrackedJourney mongoTrackedJourney = Persistence.trackedJourneys.getById(startTrackingResponse.journeyId); assertEquals(TERMINATED_BY_USER, mongoTrackedJourney.endCondition); - assertNotNull(mongoTrackedJourney.totalDeviation); - assertNotEquals(0.0, mongoTrackedJourney.totalDeviation); + assertNotEquals(-1, mongoTrackedJourney.longestConsecutiveDeviatedPoints); DateTimeUtils.useSystemDefaultClockAndTimezone(); } @@ -409,8 +408,7 @@ void canForciblyEndJourney() throws Exception { // Check that the TrackedJourney Mongo record has been updated. TrackedJourney mongoTrackedJourney = Persistence.trackedJourneys.getById(startTrackingResponse.journeyId); assertEquals(FORCIBLY_TERMINATED, mongoTrackedJourney.endCondition); - assertNotNull(mongoTrackedJourney.totalDeviation); - assertNotEquals(0.0, mongoTrackedJourney.totalDeviation); + assertNotEquals(-1, mongoTrackedJourney.longestConsecutiveDeviatedPoints); } @Test diff --git a/src/test/java/org/opentripplanner/middleware/models/TrackedJourneyTest.java b/src/test/java/org/opentripplanner/middleware/models/TrackedJourneyTest.java index 974000ccc..a5fbc417c 100644 --- a/src/test/java/org/opentripplanner/middleware/models/TrackedJourneyTest.java +++ b/src/test/java/org/opentripplanner/middleware/models/TrackedJourneyTest.java @@ -10,23 +10,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; class TrackedJourneyTest { - @Test - void canComputeTotalDeviation() { - TrackedJourney journey = new TrackedJourney(); - journey.locations = null; - assertEquals(-1.0, journey.computeTotalDeviation()); - - journey.locations = Stream - .of(11.0, 23.0, 6.4) - .map(d -> { - TrackingLocation location = new TrackingLocation(); - location.deviationMeters = d; - return location; - }) - .collect(Collectors.toList()); - assertEquals(40.4, journey.computeTotalDeviation()); - } - @Test void canComputeLargestConsecutiveDeviations() { TrackedJourney journey = new TrackedJourney(); @@ -34,18 +17,23 @@ void canComputeLargestConsecutiveDeviations() { assertEquals(-1, journey.computeLargestConsecutiveDeviations()); journey.locations = Stream - .of(0, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 0) + .of(0, 1, 1, 1, null, 1, 1, 0, 0, null, 1, 1, 1, null, 0) .map(d -> { TrackingLocation location = new TrackingLocation(); - location.tripStatus = d == 1 ? TripStatus.DEVIATED : TripStatus.ON_SCHEDULE; + location.tripStatus = d == null + ? null + : d == 1 + ? TripStatus.DEVIATED + : TripStatus.ON_SCHEDULE; location.speed = 1; return location; }) .collect(Collectors.toList()); // Insert a location where the traveler is not moving. - journey.locations.get(4).speed = 0; + journey.locations.get(5).speed = 0; + // After excluding the nulls, count the first group of consecutive ones. assertEquals(4, journey.computeLargestConsecutiveDeviations()); } } diff --git a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java index a37a9929e..a2076d5b0 100644 --- a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java +++ b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java @@ -123,7 +123,6 @@ private static TrackedJourney createJourney( journey.endCondition = endCondition; journey.endTime = endTime != null ? Date.from(endTime) : null; journey.startTime = journey.endTime; - journey.totalDeviation = (double)points; journey.longestConsecutiveDeviatedPoints = points; Persistence.trackedJourneys.create(journey); return journey; @@ -148,21 +147,6 @@ void canMapJourneysToUsers() { } } - @Test - void canSelectMostDeviatedJourney() { - TrackedJourney journey1 = new TrackedJourney(); - journey1.totalDeviation = 250.0; - journey1.endTime = Date.from(Instant.now().minus(3, ChronoUnit.HOURS)); - - TrackedJourney journey2 = new TrackedJourney(); - journey2.totalDeviation = 400.0; - journey2.endTime = Date.from(Instant.now().minus(5, ChronoUnit.HOURS)); - - Optional optJourney = TripSurveySenderJob.selectMostDeviatedJourney(List.of(journey1, journey2)); - assertTrue(optJourney.isPresent()); - assertEquals(journey2, optJourney.get()); - } - @Test void canSelectMostDeviatedJourneyUsingDeviatedPoints() { TrackedJourney journey1 = new TrackedJourney(); From eb59c119cad8ae343db58827d8f377ce6f06f599 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 25 Oct 2024 15:47:18 -0400 Subject: [PATCH 18/39] fix(TripSurveySenderJob): Send survey notifications within 30 mins of trip completion. --- .../middleware/OtpMiddlewareMain.java | 6 ++--- .../triptracker/TripSurveySenderJob.java | 14 ++++++------ .../triptracker/TripSurveySenderJobTest.java | 22 +++++++++---------- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java b/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java index 55486f74c..a07babccc 100644 --- a/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java +++ b/src/main/java/org/opentripplanner/middleware/OtpMiddlewareMain.java @@ -86,14 +86,14 @@ public static void main(String[] args) throws IOException, InterruptedException TimeUnit.MINUTES ); - // Schedule recurring job for post-trip surveys, once every hour to catch recently completed trips. + // Schedule recurring job for post-trip surveys, once every half-hour to catch recently completed trips. // TODO: Determine whether this should go in some other process. TripSurveySenderJob tripSurveySenderJob = new TripSurveySenderJob(); Scheduler.scheduleJob( tripSurveySenderJob, 0, - 1, - TimeUnit.HOURS + 30, + TimeUnit.MINUTES ); } } diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java index 2638ce15c..fa3e8f628 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java @@ -49,7 +49,7 @@ public void run() { List usersWithNotificationsOverAWeekAgo = getUsersWithNotificationsOverAWeekAgo(); // Collect journeys that were completed/terminated in the past 24-48 hrs. (skip ongoing journeys). - List journeysCompletedInPast24To48Hours = getCompletedJourneysInPast24To48Hours(); + List journeysCompletedInPast24To48Hours = getCompletedJourneysInPastHour(); // Map users to journeys. Map> usersToJourneys = mapJourneysToUsers(journeysCompletedInPast24To48Hours, usersWithNotificationsOverAWeekAgo); @@ -88,14 +88,14 @@ public static List getUsersWithNotificationsOverAWeekAgo() { } /** - * Gets tracked journeys for all users that were completed in the past 24 hours. + * Gets tracked journeys for all users that were completed in the past hour. */ - public static List getCompletedJourneysInPast24To48Hours() { - Date twentyFourHoursAgo = Date.from(Instant.now().minus(24, ChronoUnit.HOURS)); - Date fortyEightHoursAgo = Date.from(Instant.now().minus(48, ChronoUnit.HOURS)); + public static List getCompletedJourneysInPastHour() { + Date now = new Date(); + Date oneHourAgo = Date.from(Instant.now().minus(1, ChronoUnit.HOURS)); Bson dateFilter = Filters.and( - Filters.gte(END_TIME_FIELD_NAME, fortyEightHoursAgo), - Filters.lte(END_TIME_FIELD_NAME, twentyFourHoursAgo) + Filters.gte(END_TIME_FIELD_NAME, oneHourAgo), + Filters.lte(END_TIME_FIELD_NAME, now) ); Bson completeFilter = Filters.eq(END_CONDITION_FIELD_NAME, TERMINATED_BY_USER); Bson terminatedFilter = Filters.eq(END_CONDITION_FIELD_NAME, FORCIBLY_TERMINATED); diff --git a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java index a2076d5b0..4617665c6 100644 --- a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java +++ b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java @@ -102,11 +102,11 @@ void canGetUsersWithNotificationsOverAWeekAgo() { } @Test - void canGetCompletedJourneysInPast24To48Hours() { + void canGetCompletedJourneysInPastHour() { assumeTrue(IS_END_TO_END); createTestJourneys(); - List completedJourneys = TripSurveySenderJob.getCompletedJourneysInPast24To48Hours(); + List completedJourneys = TripSurveySenderJob.getCompletedJourneysInPastHour(); assertEquals(2, completedJourneys.size()); } @@ -199,8 +199,8 @@ void canRunJob() { } private static void createTestJourneys() { - Instant threeHoursAgo = Instant.now().minus(3, ChronoUnit.HOURS); - Instant thirtyHoursAgo = Instant.now().minus(30, ChronoUnit.HOURS); + Instant threeMinutesInFuture = Instant.now().plus(3, ChronoUnit.MINUTES); + Instant thirtyMinutesAgo = Instant.now().minus(30, ChronoUnit.MINUTES); Instant threeDaysAgo = Instant.now().minus(3, ChronoUnit.DAYS); // Create journey for each trip for all users above (they will be deleted explicitly after each test). @@ -208,17 +208,17 @@ private static void createTestJourneys() { // Ongoing journey (should not be included) createJourney("ongoing-journey", trip.id, null, null, 10), - // Journey completed by user 30 hours ago (should be included) - createJourney("user-terminated-journey", trip.id, thirtyHoursAgo, TERMINATED_BY_USER, 200), + // Journey completed by user 30 minutes ago (should be included) + createJourney("user-terminated-journey", trip.id, thirtyMinutesAgo, TERMINATED_BY_USER, 200), - // Journey terminated forcibly 30 hours ago (should be included) - createJourney("forcibly-terminated-journey", trip.id, thirtyHoursAgo, FORCIBLY_TERMINATED, 400), + // Journey terminated forcibly 30 minutes ago (should be included) + createJourney("forcibly-terminated-journey", trip.id, thirtyMinutesAgo, FORCIBLY_TERMINATED, 400), - // Additional journey completed over 48 hours (should not be included). + // Additional journey completed over an hour ago (should not be included). createJourney("journey-done-3-days-ago", trip.id, threeDaysAgo, TERMINATED_BY_USER, 10), - // Additional journey completed within 24 hours (should not be included). - createJourney("journey-done-3-hours-ago", trip.id, threeHoursAgo, TERMINATED_BY_USER, 10), + // Additional journey completed with bogus time in future (should not be included). + createJourney("journey-done-3-hours-ago", trip.id, threeMinutesInFuture, TERMINATED_BY_USER, 10), // Orphan journeys (should not be included) createJourney("journey-3", "other-trip", null, null, 10), From cd2b05ac6156744499d4ac4618bd977d078d59d5 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 29 Oct 2024 11:00:24 -0400 Subject: [PATCH 19/39] docs: Introduce new TRIP_SURVEY_ID config parameter. --- README.md | 1 + configurations/default/env.yml.tmp | 3 +++ src/main/resources/env.schema.json | 5 +++++ 3 files changed, 9 insertions(+) diff --git a/README.md b/README.md index a605a6943..ec5c1a3b3 100644 --- a/README.md +++ b/README.md @@ -300,6 +300,7 @@ The special E2E client settings should be defined in `env.yml`: | TRIP_TRACKING_TRAM_ON_TRACK_RADIUS | integer | Optional | 100 | The threshold in meters below which travelling by tram is considered on track. | | TRIP_INSTRUCTION_IMMEDIATE_RADIUS | integer | Optional | 2 | The radius in meters under which an immediate instruction is given. | | TRIP_INSTRUCTION_UPCOMING_RADIUS | integer | Optional | 10 | The radius in meters under which an upcoming instruction is given. | +| TRIP_SURVEY_ID | string | Optional | abcdef123y | The ID of a survey (on the platform of your choice) for trip-related feedback. | | TWILIO_ACCOUNT_SID | string | Optional | your-account-sid | Twilio settings available at: https://twilio.com/user/account | | TWILIO_AUTH_TOKEN | string | Optional | your-auth-token | Twilio settings available at: https://twilio.com/user/account | | US_RIDE_GWINNETT_BUS_OPERATOR_NOTIFIER_API_URL | string | Optional | http://host.example.com | US RideGwinnett bus notifier API. | diff --git a/configurations/default/env.yml.tmp b/configurations/default/env.yml.tmp index 93730bf49..32b691026 100644 --- a/configurations/default/env.yml.tmp +++ b/configurations/default/env.yml.tmp @@ -100,6 +100,9 @@ TRIP_INSTRUCTION_IMMEDIATE_RADIUS: 2 # The radius in meters under which an upcoming instruction is given. TRIP_INSTRUCTION_UPCOMING_RADIUS: 10 +# Survey ID that is linked to a particular trip. +TRIP_SURVEY_ID: abcdef123y + US_RIDE_GWINNETT_BUS_OPERATOR_NOTIFIER_API_URL: https://bus.notifier.example.com US_RIDE_GWINNETT_BUS_OPERATOR_NOTIFIER_API_KEY: your-key US_RIDE_GWINNETT_BUS_OPERATOR_NOTIFIER_QUALIFYING_ROUTES: agency_id:route_id diff --git a/src/main/resources/env.schema.json b/src/main/resources/env.schema.json index fcc9f79f8..afa20b2a5 100644 --- a/src/main/resources/env.schema.json +++ b/src/main/resources/env.schema.json @@ -319,6 +319,11 @@ "examples": ["10"], "description": "The radius in meters under which an upcoming instruction is given." }, + "TRIP_SURVEY_ID": { + "type": "string", + "examples": ["abcdef123y"], + "description": "The ID of a survey (on the platform of your choice) for trip-related feedback." + }, "TWILIO_ACCOUNT_SID": { "type": "string", "examples": ["your-account-sid"], From b27a4dda35cdf675882e90ed8d23976a6bf16601 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 29 Oct 2024 14:15:21 -0400 Subject: [PATCH 20/39] refactor(Message): Extract i18n message for trip survey notification, delete FTL template. --- .../org/opentripplanner/middleware/i18n/Message.java | 3 ++- src/main/resources/Message.properties | 3 ++- src/main/resources/Message_fr.properties | 3 ++- src/main/resources/templates/PostTripSurveyPush.ftl | 9 --------- 4 files changed, 6 insertions(+), 12 deletions(-) delete mode 100644 src/main/resources/templates/PostTripSurveyPush.ftl diff --git a/src/main/java/org/opentripplanner/middleware/i18n/Message.java b/src/main/java/org/opentripplanner/middleware/i18n/Message.java index d3946e31f..efa223641 100644 --- a/src/main/java/org/opentripplanner/middleware/i18n/Message.java +++ b/src/main/java/org/opentripplanner/middleware/i18n/Message.java @@ -40,7 +40,8 @@ public enum Message { TRIP_DELAY_MINUTES, TRIP_NOT_FOUND_NOTIFICATION, TRIP_NO_LONGER_POSSIBLE_NOTIFICATION, - TRIP_REMINDER_NOTIFICATION; + TRIP_REMINDER_NOTIFICATION, + TRIP_SURVEY_NOTIFICATION; private static final Logger LOG = LoggerFactory.getLogger(Message.class); diff --git a/src/main/resources/Message.properties b/src/main/resources/Message.properties index 2f3a461b4..200c101e1 100644 --- a/src/main/resources/Message.properties +++ b/src/main/resources/Message.properties @@ -25,4 +25,5 @@ TRIP_DELAY_LATE = %s late TRIP_DELAY_MINUTES = %d minutes TRIP_NOT_FOUND_NOTIFICATION = Your itinerary was not found in today's trip planner results. Please check real-time conditions and plan a new trip. TRIP_NO_LONGER_POSSIBLE_NOTIFICATION = Your itinerary is no longer possible on any monitored day of the week. Please plan and save a new trip. -TRIP_REMINDER_NOTIFICATION = Reminder for %s at %s. \ No newline at end of file +TRIP_REMINDER_NOTIFICATION = Reminder for %s at %s. +TRIP_SURVEY_NOTIFICATION = How was your most recent trip starting at %s? Tap for a quick survey. \ No newline at end of file diff --git a/src/main/resources/Message_fr.properties b/src/main/resources/Message_fr.properties index b64acce24..5c3af556d 100644 --- a/src/main/resources/Message_fr.properties +++ b/src/main/resources/Message_fr.properties @@ -25,4 +25,5 @@ TRIP_DELAY_LATE = %s en retard TRIP_DELAY_MINUTES = %d minutes TRIP_NOT_FOUND_NOTIFICATION = Votre trajet est introuvable aujourd'hui. Veuillez vérifier les conditions en temps-réel et recherchez un nouveau trajet. TRIP_NO_LONGER_POSSIBLE_NOTIFICATION = Votre trajet n'est plus possible dans aucun jour de suivi de la semaine. Veuillez rechercher et enregistrer un nouveau trajet. -TRIP_REMINDER_NOTIFICATION = Rappel pour %s à %s. \ No newline at end of file +TRIP_REMINDER_NOTIFICATION = Rappel pour %s à %s. +TRIP_SURVEY_NOTIFICATION = Comment s'est passé votre trajet de %s ? Tapez pour un sondage rapide. \ No newline at end of file diff --git a/src/main/resources/templates/PostTripSurveyPush.ftl b/src/main/resources/templates/PostTripSurveyPush.ftl deleted file mode 100644 index c6501ee2d..000000000 --- a/src/main/resources/templates/PostTripSurveyPush.ftl +++ /dev/null @@ -1,9 +0,0 @@ -<#-- - This is a template for push notifications content for notifications - after an OTP user takes a monitored trip and completes travel. - Note the following character limitations by mobile OS: - - iOS: 178 characters over up to 4 lines, - - Android: 240 characters (excluding notification title). - The max length is thus 178 characters. ---> -How was your most recent trip starting at ${tripTime}? Tap for a quick survey. From 4cd55b0d5a75ea05d527ddce4629ba23ede111f9 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 29 Oct 2024 14:52:39 -0400 Subject: [PATCH 21/39] refactor(TripSurveySenderJob): Streamline notification for trip surveys. --- .../triptracker/TripSurveySenderJob.java | 21 +++++++++---------- .../middleware/utils/NotificationUtils.java | 20 ++++++++++++++++++ 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java index fa3e8f628..4bf50be25 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java @@ -6,8 +6,6 @@ import org.opentripplanner.middleware.models.OtpUser; import org.opentripplanner.middleware.models.TrackedJourney; import org.opentripplanner.middleware.persistence.Persistence; -import org.opentripplanner.middleware.utils.DateTimeUtils; -import org.opentripplanner.middleware.utils.I18nUtils; import org.opentripplanner.middleware.utils.NotificationUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -59,16 +57,17 @@ public void run() { Optional optJourney = selectMostDeviatedJourneyUsingDeviatedPoints(entry.getValue()); if (optJourney.isPresent()) { // Send push notification about that journey. + MonitoredTrip trip = optJourney.get().trip; + LOG.info("Sending survey notification for trip {}", trip.id); OtpUser otpUser = entry.getKey(); - TrackedJourney journey = optJourney.get(); - MonitoredTrip trip = journey.trip; - Map data = Map.of( - "tripTime", DateTimeUtils.formatShortDate(trip.itinerary.startTime, I18nUtils.getOtpUserLocale(otpUser)) - ); - NotificationUtils.sendPush(otpUser, "PostTripSurveyPush.ftl", data, trip.tripName, trip.id); - - // Store time of last sent survey notification for user. - Persistence.otpUsers.updateField(otpUser.id, LAST_TRIP_SURVEY_NOTIF_SENT_FIELD, new Date()); + String pushResult = NotificationUtils.sendTripSurveyPush(otpUser, trip); + if (pushResult != null) { + // Store time of last sent survey notification for user. + Persistence.otpUsers.updateField(otpUser.id, LAST_TRIP_SURVEY_NOTIF_SENT_FIELD, new Date()); + } else { + LOG.warn("Could not send survey notification for trip {}", trip.id); + } + } } diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index e09bf18fc..cf2647973 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -15,6 +15,7 @@ import org.opentripplanner.middleware.bugsnag.BugsnagReporter; import org.opentripplanner.middleware.models.AdminUser; import org.opentripplanner.middleware.models.Device; +import org.opentripplanner.middleware.models.MonitoredTrip; import org.opentripplanner.middleware.models.OtpUser; import org.opentripplanner.middleware.persistence.Persistence; import org.slf4j.Logger; @@ -24,6 +25,7 @@ import java.net.URI; import java.net.URLEncoder; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Objects; import java.util.stream.Collectors; @@ -86,6 +88,24 @@ public static String sendPush(OtpUser otpUser, String textTemplate, Object templ } } + /** + * @param otpUser target user + * @param trip Trip about which the survey notification is about. + */ + public static String sendTripSurveyPush(OtpUser otpUser, MonitoredTrip trip) { + // If Push API config properties aren't set, do nothing. + if (PUSH_API_KEY == null || PUSH_API_URL == null) return null; + + Locale locale = I18nUtils.getOtpUserLocale(otpUser); + String tripTime = DateTimeUtils.formatShortDate(trip.itinerary.startTime, locale); + String body = String.format( + org.opentripplanner.middleware.i18n.Message.TRIP_SURVEY_NOTIFICATION.get(locale), + tripTime + ); + String toUser = otpUser.email; + return otpUser.pushDevices > 0 ? sendPush(toUser, body, trip.tripName, trip.id) : "OK"; + } + /** * Send a push notification message to the provided user * @param toUser user account ID (email address) From 740fdc7c0de37f20bf68119bd63ca8d0fc0f577e Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 29 Oct 2024 15:31:13 -0400 Subject: [PATCH 22/39] refactor(NotificationUtils): Add surveyId and userId fields and rework ctors. --- .../middleware/utils/NotificationUtils.java | 36 ++++++++++++++----- .../utils/NotificationUtilsTest.java | 7 +++- .../utils/NotificationUtilsTestCI.java | 25 ++++++------- 3 files changed, 44 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index cf2647973..d01914605 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -79,8 +79,7 @@ public static String sendPush(OtpUser otpUser, String textTemplate, Object templ if (PUSH_API_KEY == null || PUSH_API_URL == null) return null; try { String body = TemplateUtils.renderTemplate(textTemplate, templateData); - String toUser = otpUser.email; - return otpUser.pushDevices > 0 ? sendPush(toUser, body, tripName, tripId) : "OK"; + return otpUser.pushDevices > 0 ? sendPush(otpUser, body, tripName, tripId) : "OK"; } catch (TemplateException | IOException e) { // This catch indicates there was an error rendering the template. Note: TemplateUtils#renderTemplate // handles Bugsnag reporting/error logging, so that is not needed here. @@ -102,26 +101,26 @@ public static String sendTripSurveyPush(OtpUser otpUser, MonitoredTrip trip) { org.opentripplanner.middleware.i18n.Message.TRIP_SURVEY_NOTIFICATION.get(locale), tripTime ); - String toUser = otpUser.email; - return otpUser.pushDevices > 0 ? sendPush(toUser, body, trip.tripName, trip.id) : "OK"; + return otpUser.pushDevices > 0 ? sendPush(otpUser, body, trip.tripName, trip.id) : "OK"; } /** * Send a push notification message to the provided user * @param toUser user account ID (email address) * @param body message body + * @param tripName Monitored trip name to show in notification title * @param tripId Monitored trip ID * @return "OK" if message was successful (null otherwise) */ - static String sendPush(String toUser, String body, String tripName, String tripId) { + static String sendPush(OtpUser toUser, String body, String tripName, String tripId) { try { - NotificationInfo notifInfo = new NotificationInfo( + NotificationInfo notificationInfo = new NotificationInfo( toUser, body, tripName, tripId ); - var jsonBody = new Gson().toJson(notifInfo); + var jsonBody = new Gson().toJson(notificationInfo); var httpResponse = HttpUtils.httpRequestRawResponse( URI.create(PUSH_API_URL + "/notification/publish?api_key=" + PUSH_API_KEY), 1000, @@ -414,19 +413,38 @@ public static void updatePushDevices(OtpUser otpUser) { } static class NotificationInfo { + /** In reality, the email of the desired user (the push service we use looks up users by email) */ public final String user; + + /** The Mongo ID of the desired user */ + public final String userId; + + /** The message shown in the notification body */ public final String message; + + /** The title of this notification */ public final String title; + + /** The ID of the trip associated to this notification */ public final String tripId; - public NotificationInfo(String user, String message, String title, String tripId) { + /** The ID of the survey to be launched for said trip, if applicable. */ + public final String surveyId; + + public NotificationInfo(OtpUser user, String message, String title, String tripId) { + this(user, message, title, tripId, null); + } + + public NotificationInfo(OtpUser user, String message, String title, String tripId, String surveyId) { String truncatedTitle = StringUtil.truncate(title, PUSH_TITLE_MAX_LENGTH); int truncatedMessageLength = PUSH_TOTAL_MAX_LENGTH - truncatedTitle.length(); - this.user = user; + this.user = user.email; + this.userId = user.id; this.title = truncatedTitle; this.message = StringUtil.truncate(message, truncatedMessageLength); this.tripId = tripId; + this.surveyId = surveyId; } } } diff --git a/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java b/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java index ec3a48b2a..1df4f7c56 100644 --- a/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java +++ b/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java @@ -8,6 +8,7 @@ import org.junit.jupiter.params.provider.NullSource; import org.junit.jupiter.params.provider.ValueSource; import org.opentripplanner.middleware.models.Device; +import org.opentripplanner.middleware.models.OtpUser; import java.util.List; import java.util.stream.Stream; @@ -85,8 +86,12 @@ void testNumberOfUniqueDevices() { @ParameterizedTest @MethodSource("createNotificationInfoCases") void testTruncateNotificationPayload(String originalTitle, String expectedTitle, String originalMessage, String expectedMessage, String message) { + OtpUser user = new OtpUser(); + user.id = "user123u"; + user.email = "user@example.com"; + NotificationUtils.NotificationInfo info = new NotificationUtils.NotificationInfo( - "user@example.com", + user, originalMessage, originalTitle, "trip-id" diff --git a/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTestCI.java b/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTestCI.java index 81d9d06d4..41438a650 100644 --- a/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTestCI.java +++ b/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTestCI.java @@ -12,7 +12,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.IOException; import java.util.Date; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -25,7 +24,7 @@ * Note: these tests require the environment variables RUN_E2E=true and valid values for TEST_TO_EMAIL, TEST_TO_PHONE, * and TEST_TO_PUSH. Furthermore, TEST_TO_PHONE must be a verified phone number in a valid Twilio account. */ -public class NotificationUtilsTestCI extends OtpMiddlewareTestEnvironment { +class NotificationUtilsTestCI extends OtpMiddlewareTestEnvironment { private static final Logger LOG = LoggerFactory.getLogger(NotificationUtilsTestCI.class); private static OtpUser user; @@ -36,16 +35,15 @@ public class NotificationUtilsTestCI extends OtpMiddlewareTestEnvironment { private static final String email = System.getenv("TEST_TO_EMAIL"); /** Phone must be in the form "+15551234" and must be verified first in order to send notifications */ private static final String phone = System.getenv("TEST_TO_PHONE"); - /** Push notification is conventionally a user.email value and must be known to the mobile team's push API */ - private static final String push = System.getenv("TEST_TO_PUSH"); + /** * Currently, since these tests require target email/SMS values, these tests should not run on CI. */ private static final boolean shouldTestsRun = - !isRunningCi && IS_END_TO_END && email != null && phone != null && push != null; + !isRunningCi && IS_END_TO_END && email != null && phone != null; @BeforeAll - public static void setup() throws IOException { + public static void setup() { assumeTrue(shouldTestsRun); user = createUser(email, phone); } @@ -56,20 +54,19 @@ public static void tearDown() { } @Test - public void canSendPushNotification() { + void canSendPushNotification() { String ret = NotificationUtils.sendPush( - // Conventionally user.email - push, + user, "Tough little ship!", "Titanic", "trip-id" ); - LOG.info("Push notification (ret={}) sent to {}", ret, push); + LOG.info("Push notification (ret={}) sent to {}", ret, user.email); Assertions.assertNotNull(ret); } @Test - public void canSendSparkpostEmailNotification() { + void canSendSparkpostEmailNotification() { boolean success = NotificationUtils.sendEmailViaSparkpost( OTP_ADMIN_DASHBOARD_FROM_EMAIL, user.email, @@ -81,7 +78,7 @@ public void canSendSparkpostEmailNotification() { } @Test - public void canSendSmsNotification() { + void canSendSmsNotification() { // Note: toPhone must be verified. String messageId = NotificationUtils.sendSMS( // Note: phone number is configured in setup method above. @@ -96,7 +93,7 @@ public void canSendSmsNotification() { * Tests whether a verification code can be sent to a phone number. */ @Test - public void canSendTwilioVerificationText() { + void canSendTwilioVerificationText() { Assertions.assertNull(user.smsConsentDate); Date beforeVerificationDate = new Date(); Verification verification = NotificationUtils.sendVerificationText( @@ -117,7 +114,7 @@ public void canSendTwilioVerificationText() { * your phone can be used below (in place of 123456) to generate an "approved" status. */ @Test - public void canCheckSmsVerificationCode() { + void canCheckSmsVerificationCode() { VerificationCheck check = NotificationUtils.checkSmsVerificationCode( // Note: phone number is configured in setup method above. user.phoneNumber, From 6c5548199d3824b864cd524626e8d35e9776f9e0 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 29 Oct 2024 15:43:39 -0400 Subject: [PATCH 23/39] feat(NotificationUtils): Pass surveyId with trip survey notifications. --- .../middleware/utils/NotificationUtils.java | 11 +++++++---- .../middleware/utils/NotificationUtilsTestCI.java | 3 ++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index d01914605..9a224e0b5 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -52,6 +52,7 @@ public class NotificationUtils { public static final String OTP_ADMIN_DASHBOARD_FROM_EMAIL = getConfigPropertyAsText("OTP_ADMIN_DASHBOARD_FROM_EMAIL"); private static final String PUSH_API_KEY = getConfigPropertyAsText("PUSH_API_KEY"); private static final String PUSH_API_URL = getConfigPropertyAsText("PUSH_API_URL"); + private static final String TRIP_SURVEY_ID = getConfigPropertyAsText("TRIP_SURVEY_ID"); /** * Although SMS are 160 characters long and Twilio supports sending up to 1600 characters, @@ -79,7 +80,7 @@ public static String sendPush(OtpUser otpUser, String textTemplate, Object templ if (PUSH_API_KEY == null || PUSH_API_URL == null) return null; try { String body = TemplateUtils.renderTemplate(textTemplate, templateData); - return otpUser.pushDevices > 0 ? sendPush(otpUser, body, tripName, tripId) : "OK"; + return otpUser.pushDevices > 0 ? sendPush(otpUser, body, tripName, tripId, null) : "OK"; } catch (TemplateException | IOException e) { // This catch indicates there was an error rendering the template. Note: TemplateUtils#renderTemplate // handles Bugsnag reporting/error logging, so that is not needed here. @@ -101,7 +102,7 @@ public static String sendTripSurveyPush(OtpUser otpUser, MonitoredTrip trip) { org.opentripplanner.middleware.i18n.Message.TRIP_SURVEY_NOTIFICATION.get(locale), tripTime ); - return otpUser.pushDevices > 0 ? sendPush(otpUser, body, trip.tripName, trip.id) : "OK"; + return otpUser.pushDevices > 0 ? sendPush(otpUser, body, trip.tripName, trip.id, TRIP_SURVEY_ID) : "OK"; } /** @@ -110,15 +111,17 @@ public static String sendTripSurveyPush(OtpUser otpUser, MonitoredTrip trip) { * @param body message body * @param tripName Monitored trip name to show in notification title * @param tripId Monitored trip ID + * @param surveyId Survey ID * @return "OK" if message was successful (null otherwise) */ - static String sendPush(OtpUser toUser, String body, String tripName, String tripId) { + static String sendPush(OtpUser toUser, String body, String tripName, String tripId, String surveyId) { try { NotificationInfo notificationInfo = new NotificationInfo( toUser, body, tripName, - tripId + tripId, + surveyId ); var jsonBody = new Gson().toJson(notificationInfo); var httpResponse = HttpUtils.httpRequestRawResponse( diff --git a/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTestCI.java b/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTestCI.java index 41438a650..00a421980 100644 --- a/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTestCI.java +++ b/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTestCI.java @@ -59,7 +59,8 @@ void canSendPushNotification() { user, "Tough little ship!", "Titanic", - "trip-id" + "trip-id", + "survey-id" ); LOG.info("Push notification (ret={}) sent to {}", ret, user.email); Assertions.assertNotNull(ret); From 90d1f4b69f158ebdf7ec249229804f99658c11b3 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 29 Oct 2024 15:57:42 -0400 Subject: [PATCH 24/39] docs(swagger): Update snapshot. --- .../latest-spark-swagger-output.yaml | 232 +++++++++--------- 1 file changed, 116 insertions(+), 116 deletions(-) diff --git a/src/main/resources/latest-spark-swagger-output.yaml b/src/main/resources/latest-spark-swagger-output.yaml index f2a924cfb..068002f93 100644 --- a/src/main/resources/latest-spark-swagger-output.yaml +++ b/src/main/resources/latest-spark-swagger-output.yaml @@ -56,10 +56,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/AdminUser" responseSchema: $ref: "#/definitions/AdminUser" + schema: + $ref: "#/definitions/AdminUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -90,10 +90,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/AdminUser" responseSchema: $ref: "#/definitions/AdminUser" + schema: + $ref: "#/definitions/AdminUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -123,10 +123,10 @@ paths: responses: "200": description: "successful operation" - schema: - $ref: "#/definitions/Job" responseSchema: $ref: "#/definitions/Job" + schema: + $ref: "#/definitions/Job" /api/admin/user: get: tags: @@ -155,10 +155,10 @@ paths: responses: "200": description: "successful operation" - schema: - $ref: "#/definitions/ResponseList" responseSchema: $ref: "#/definitions/ResponseList" + schema: + $ref: "#/definitions/ResponseList" post: tags: - "api/admin/user" @@ -178,10 +178,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/AdminUser" responseSchema: $ref: "#/definitions/AdminUser" + schema: + $ref: "#/definitions/AdminUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -220,10 +220,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/AdminUser" responseSchema: $ref: "#/definitions/AdminUser" + schema: + $ref: "#/definitions/AdminUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -269,10 +269,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/AdminUser" responseSchema: $ref: "#/definitions/AdminUser" + schema: + $ref: "#/definitions/AdminUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -309,10 +309,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/AdminUser" responseSchema: $ref: "#/definitions/AdminUser" + schema: + $ref: "#/definitions/AdminUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -356,10 +356,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/ApiUser" responseSchema: $ref: "#/definitions/ApiUser" + schema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -402,10 +402,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/ApiUser" responseSchema: $ref: "#/definitions/ApiUser" + schema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -447,10 +447,10 @@ paths: responses: "200": description: "successful operation" - schema: - $ref: "#/definitions/TokenHolder" responseSchema: $ref: "#/definitions/TokenHolder" + schema: + $ref: "#/definitions/TokenHolder" /api/secure/application/fromtoken: get: tags: @@ -464,10 +464,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/ApiUser" responseSchema: $ref: "#/definitions/ApiUser" + schema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -498,10 +498,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/ApiUser" responseSchema: $ref: "#/definitions/ApiUser" + schema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -531,10 +531,10 @@ paths: responses: "200": description: "successful operation" - schema: - $ref: "#/definitions/Job" responseSchema: $ref: "#/definitions/Job" + schema: + $ref: "#/definitions/Job" /api/secure/application: get: tags: @@ -563,10 +563,10 @@ paths: responses: "200": description: "successful operation" - schema: - $ref: "#/definitions/ResponseList" responseSchema: $ref: "#/definitions/ResponseList" + schema: + $ref: "#/definitions/ResponseList" post: tags: - "api/secure/application" @@ -586,10 +586,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/ApiUser" responseSchema: $ref: "#/definitions/ApiUser" + schema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -628,10 +628,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/ApiUser" responseSchema: $ref: "#/definitions/ApiUser" + schema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -677,10 +677,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/ApiUser" responseSchema: $ref: "#/definitions/ApiUser" + schema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -717,10 +717,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/ApiUser" responseSchema: $ref: "#/definitions/ApiUser" + schema: + $ref: "#/definitions/ApiUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -754,10 +754,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/CDPUser" responseSchema: $ref: "#/definitions/CDPUser" + schema: + $ref: "#/definitions/CDPUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -788,10 +788,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/CDPUser" responseSchema: $ref: "#/definitions/CDPUser" + schema: + $ref: "#/definitions/CDPUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -821,10 +821,10 @@ paths: responses: "200": description: "successful operation" - schema: - $ref: "#/definitions/Job" responseSchema: $ref: "#/definitions/Job" + schema: + $ref: "#/definitions/Job" /api/secure/cdp: get: tags: @@ -853,10 +853,10 @@ paths: responses: "200": description: "successful operation" - schema: - $ref: "#/definitions/ResponseList" responseSchema: $ref: "#/definitions/ResponseList" + schema: + $ref: "#/definitions/ResponseList" post: tags: - "api/secure/cdp" @@ -876,10 +876,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/CDPUser" responseSchema: $ref: "#/definitions/CDPUser" + schema: + $ref: "#/definitions/CDPUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -918,10 +918,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/CDPUser" responseSchema: $ref: "#/definitions/CDPUser" + schema: + $ref: "#/definitions/CDPUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -967,10 +967,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/CDPUser" responseSchema: $ref: "#/definitions/CDPUser" + schema: + $ref: "#/definitions/CDPUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1007,10 +1007,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/CDPUser" responseSchema: $ref: "#/definitions/CDPUser" + schema: + $ref: "#/definitions/CDPUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1050,10 +1050,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/ItineraryExistence" responseSchema: $ref: "#/definitions/ItineraryExistence" + schema: + $ref: "#/definitions/ItineraryExistence" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1102,10 +1102,10 @@ paths: responses: "200": description: "successful operation" - schema: - $ref: "#/definitions/ResponseList" responseSchema: $ref: "#/definitions/ResponseList" + schema: + $ref: "#/definitions/ResponseList" post: tags: - "api/secure/monitoredtrip" @@ -1125,10 +1125,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/MonitoredTrip" responseSchema: $ref: "#/definitions/MonitoredTrip" + schema: + $ref: "#/definitions/MonitoredTrip" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1167,10 +1167,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/MonitoredTrip" responseSchema: $ref: "#/definitions/MonitoredTrip" + schema: + $ref: "#/definitions/MonitoredTrip" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1216,10 +1216,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/MonitoredTrip" responseSchema: $ref: "#/definitions/MonitoredTrip" + schema: + $ref: "#/definitions/MonitoredTrip" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1257,10 +1257,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/MonitoredTrip" responseSchema: $ref: "#/definitions/MonitoredTrip" + schema: + $ref: "#/definitions/MonitoredTrip" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1298,10 +1298,10 @@ paths: responses: "200": description: "successful operation" - schema: - $ref: "#/definitions/TrackingResponse" responseSchema: $ref: "#/definitions/TrackingResponse" + schema: + $ref: "#/definitions/TrackingResponse" /api/secure/monitoredtrip/updatetracking: post: tags: @@ -1319,10 +1319,10 @@ paths: responses: "200": description: "successful operation" - schema: - $ref: "#/definitions/TrackingResponse" responseSchema: $ref: "#/definitions/TrackingResponse" + schema: + $ref: "#/definitions/TrackingResponse" /api/secure/monitoredtrip/track: post: tags: @@ -1340,10 +1340,10 @@ paths: responses: "200": description: "successful operation" - schema: - $ref: "#/definitions/TrackingResponse" responseSchema: $ref: "#/definitions/TrackingResponse" + schema: + $ref: "#/definitions/TrackingResponse" /api/secure/monitoredtrip/endtracking: post: tags: @@ -1361,10 +1361,10 @@ paths: responses: "200": description: "successful operation" - schema: - $ref: "#/definitions/EndTrackingResponse" responseSchema: $ref: "#/definitions/EndTrackingResponse" + schema: + $ref: "#/definitions/EndTrackingResponse" /api/secure/monitoredtrip/forciblyendtracking: post: tags: @@ -1382,10 +1382,10 @@ paths: responses: "200": description: "successful operation" - schema: - $ref: "#/definitions/EndTrackingResponse" responseSchema: $ref: "#/definitions/EndTrackingResponse" + schema: + $ref: "#/definitions/EndTrackingResponse" /api/secure/triprequests: get: tags: @@ -1430,10 +1430,10 @@ paths: responses: "200": description: "successful operation" - schema: - $ref: "#/definitions/TripRequest" responseSchema: $ref: "#/definitions/TripRequest" + schema: + $ref: "#/definitions/TripRequest" /api/secure/monitoredcomponent: get: tags: @@ -1462,10 +1462,10 @@ paths: responses: "200": description: "successful operation" - schema: - $ref: "#/definitions/ResponseList" responseSchema: $ref: "#/definitions/ResponseList" + schema: + $ref: "#/definitions/ResponseList" post: tags: - "api/secure/monitoredcomponent" @@ -1485,10 +1485,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/MonitoredComponent" responseSchema: $ref: "#/definitions/MonitoredComponent" + schema: + $ref: "#/definitions/MonitoredComponent" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1527,10 +1527,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/MonitoredComponent" responseSchema: $ref: "#/definitions/MonitoredComponent" + schema: + $ref: "#/definitions/MonitoredComponent" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1576,10 +1576,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/MonitoredComponent" responseSchema: $ref: "#/definitions/MonitoredComponent" + schema: + $ref: "#/definitions/MonitoredComponent" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1617,10 +1617,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/MonitoredComponent" responseSchema: $ref: "#/definitions/MonitoredComponent" + schema: + $ref: "#/definitions/MonitoredComponent" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1660,10 +1660,10 @@ paths: responses: "200": description: "successful operation" - schema: - $ref: "#/definitions/VerificationResult" responseSchema: $ref: "#/definitions/VerificationResult" + schema: + $ref: "#/definitions/VerificationResult" /api/secure/user/{id}/verify_sms/{code}: post: tags: @@ -1683,10 +1683,10 @@ paths: responses: "200": description: "successful operation" - schema: - $ref: "#/definitions/VerificationResult" responseSchema: $ref: "#/definitions/VerificationResult" + schema: + $ref: "#/definitions/VerificationResult" /api/secure/user/fromtoken: get: tags: @@ -1700,10 +1700,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/OtpUser" responseSchema: $ref: "#/definitions/OtpUser" + schema: + $ref: "#/definitions/OtpUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1734,10 +1734,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/OtpUser" responseSchema: $ref: "#/definitions/OtpUser" + schema: + $ref: "#/definitions/OtpUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1767,10 +1767,10 @@ paths: responses: "200": description: "successful operation" - schema: - $ref: "#/definitions/Job" responseSchema: $ref: "#/definitions/Job" + schema: + $ref: "#/definitions/Job" /api/secure/user: get: tags: @@ -1799,10 +1799,10 @@ paths: responses: "200": description: "successful operation" - schema: - $ref: "#/definitions/ResponseList" responseSchema: $ref: "#/definitions/ResponseList" + schema: + $ref: "#/definitions/ResponseList" post: tags: - "api/secure/user" @@ -1822,10 +1822,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/OtpUser" responseSchema: $ref: "#/definitions/OtpUser" + schema: + $ref: "#/definitions/OtpUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1864,10 +1864,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/OtpUser" responseSchema: $ref: "#/definitions/OtpUser" + schema: + $ref: "#/definitions/OtpUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1913,10 +1913,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/OtpUser" responseSchema: $ref: "#/definitions/OtpUser" + schema: + $ref: "#/definitions/OtpUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -1953,10 +1953,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/OtpUser" responseSchema: $ref: "#/definitions/OtpUser" + schema: + $ref: "#/definitions/OtpUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -2010,11 +2010,11 @@ paths: responses: "200": description: "successful operation" - schema: + responseSchema: type: "array" items: $ref: "#/definitions/ApiUsageResult" - responseSchema: + schema: type: "array" items: $ref: "#/definitions/ApiUsageResult" @@ -2041,11 +2041,11 @@ paths: responses: "200": description: "successful operation" - schema: + responseSchema: type: "array" items: $ref: "#/definitions/BugsnagEvent" - responseSchema: + schema: type: "array" items: $ref: "#/definitions/BugsnagEvent" @@ -2072,11 +2072,11 @@ paths: responses: "200": description: "successful operation" - schema: + responseSchema: type: "array" items: $ref: "#/definitions/CDPFile" - responseSchema: + schema: type: "array" items: $ref: "#/definitions/CDPFile" @@ -2097,11 +2097,11 @@ paths: responses: "200": description: "successful operation" - schema: + responseSchema: type: "array" items: $ref: "#/definitions/URL" - responseSchema: + schema: type: "array" items: $ref: "#/definitions/URL" From b89e1dbc3952df57f7fa412de2b5b11e74dead2c Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 29 Oct 2024 16:20:25 -0400 Subject: [PATCH 25/39] refactor(TripSurveySenderJob): Rename variable. --- .../middleware/triptracker/TripSurveySenderJob.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java index 4bf50be25..7720ddca8 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java @@ -47,10 +47,10 @@ public void run() { List usersWithNotificationsOverAWeekAgo = getUsersWithNotificationsOverAWeekAgo(); // Collect journeys that were completed/terminated in the past 24-48 hrs. (skip ongoing journeys). - List journeysCompletedInPast24To48Hours = getCompletedJourneysInPastHour(); + List journeysCompletedInPastHour = getCompletedJourneysInPastHour(); // Map users to journeys. - Map> usersToJourneys = mapJourneysToUsers(journeysCompletedInPast24To48Hours, usersWithNotificationsOverAWeekAgo); + Map> usersToJourneys = mapJourneysToUsers(journeysCompletedInPastHour, usersWithNotificationsOverAWeekAgo); for (Map.Entry> entry : usersToJourneys.entrySet()) { // Find journey with the largest total deviation. From 8d92c4fd50f311a8a3162ec4ca440c73a2129ecc Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 29 Oct 2024 16:37:26 -0400 Subject: [PATCH 26/39] refactor(NotificationUtils): Check for TRIP_SURVEY_ID before attempting request. --- .../opentripplanner/middleware/utils/NotificationUtils.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index 9a224e0b5..b91a95461 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -93,8 +93,8 @@ public static String sendPush(OtpUser otpUser, String textTemplate, Object templ * @param trip Trip about which the survey notification is about. */ public static String sendTripSurveyPush(OtpUser otpUser, MonitoredTrip trip) { - // If Push API config properties aren't set, do nothing. - if (PUSH_API_KEY == null || PUSH_API_URL == null) return null; + // If Push API/survey config properties aren't set, do nothing. + if (PUSH_API_KEY == null || PUSH_API_URL == null || TRIP_SURVEY_ID == null) return null; Locale locale = I18nUtils.getOtpUserLocale(otpUser); String tripTime = DateTimeUtils.formatShortDate(trip.itinerary.startTime, locale); From 1381b067d99634ea2784984753ec8ac934b13e02 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 29 Oct 2024 16:49:52 -0400 Subject: [PATCH 27/39] refactor(NotificationUtils): Check for devices before rest of trip survey configs and sending request. --- .../middleware/utils/NotificationUtils.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index b91a95461..26409e74f 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -93,7 +93,10 @@ public static String sendPush(OtpUser otpUser, String textTemplate, Object templ * @param trip Trip about which the survey notification is about. */ public static String sendTripSurveyPush(OtpUser otpUser, MonitoredTrip trip) { - // If Push API/survey config properties aren't set, do nothing. + // Check devices first - No devices returns OK (favors E2E testing) + if (otpUser.pushDevices == 0) return "OK"; + + // If Push API/survey config properties aren't set, do nothing (will trigger warning log). if (PUSH_API_KEY == null || PUSH_API_URL == null || TRIP_SURVEY_ID == null) return null; Locale locale = I18nUtils.getOtpUserLocale(otpUser); @@ -102,7 +105,7 @@ public static String sendTripSurveyPush(OtpUser otpUser, MonitoredTrip trip) { org.opentripplanner.middleware.i18n.Message.TRIP_SURVEY_NOTIFICATION.get(locale), tripTime ); - return otpUser.pushDevices > 0 ? sendPush(otpUser, body, trip.tripName, trip.id, TRIP_SURVEY_ID) : "OK"; + return sendPush(otpUser, body, trip.tripName, trip.id, TRIP_SURVEY_ID); } /** From edb9392d3ab1c43655fe5bd3066e99ad954a16f7 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 29 Oct 2024 17:10:57 -0400 Subject: [PATCH 28/39] refactor(TripSurveySenderJob): Tweak comments. --- .../middleware/triptracker/TripSurveySenderJob.java | 2 +- .../middleware/triptracker/TripSurveySenderJobTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java index 7720ddca8..5605a9081 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java @@ -46,7 +46,7 @@ public void run() { // Pick users for which the last survey notification was sent more than a week ago. List usersWithNotificationsOverAWeekAgo = getUsersWithNotificationsOverAWeekAgo(); - // Collect journeys that were completed/terminated in the past 24-48 hrs. (skip ongoing journeys). + // Collect journeys that were completed/terminated in the past hour (skip ongoing journeys). List journeysCompletedInPastHour = getCompletedJourneysInPastHour(); // Map users to journeys. diff --git a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java index 4617665c6..8e3e4234a 100644 --- a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java +++ b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java @@ -44,7 +44,7 @@ class TripSurveySenderJobTest extends OtpMiddlewareTestEnvironment { public static void setUp() { assumeTrue(IS_END_TO_END); - // Create users. and populate the date for last trip survey notification. + // Create users and populate the date for last trip survey notification. user1notifiedNow = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("test-user1")); user2notifiedAWeekAgo = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("test-user2")); user3neverNotified = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("test-user3")); From 4755ba819391ff88ee3a13c0905545e759ef85a8 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 30 Oct 2024 13:19:09 -0400 Subject: [PATCH 29/39] refactor(TrackedJourney): Remove unused method. --- .../middleware/models/TrackedJourney.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java b/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java index 3558940b4..f328f4647 100644 --- a/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java +++ b/src/main/java/org/opentripplanner/middleware/models/TrackedJourney.java @@ -99,16 +99,6 @@ public void updateNotificationMessage(String routeId, String body) { ); } - /** The sum of the deviations for all tracking locations that have it. */ - public double computeTotalDeviation() { - if (locations == null) return -1; - - return locations.stream() - .filter(l -> l.deviationMeters != null) - .map(l -> l.deviationMeters) - .reduce(0.0, Double::sum); - } - /** The largest consecutive deviations for all tracking locations marked "deviated". */ public int computeLargestConsecutiveDeviations() { if (locations == null) return -1; From 0a8a81ba30c082f181367f75693299af48896ed9 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 31 Oct 2024 14:35:24 -0400 Subject: [PATCH 30/39] style(ManageTripTracking):Wrap line --- .../middleware/triptracker/ManageTripTracking.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java b/src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java index 9466d8527..76fdbdbcd 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/ManageTripTracking.java @@ -153,7 +153,11 @@ private static EndTrackingResponse completeJourney(TripTrackingData tripData, bo Persistence.trackedJourneys.updateField(trackedJourney.id, TrackedJourney.END_TIME_FIELD_NAME, trackedJourney.endTime); Persistence.trackedJourneys.updateField(trackedJourney.id, TrackedJourney.END_CONDITION_FIELD_NAME, trackedJourney.endCondition); trackedJourney.longestConsecutiveDeviatedPoints = trackedJourney.computeLargestConsecutiveDeviations(); - Persistence.trackedJourneys.updateField(trackedJourney.id, TrackedJourney.LONGEST_CONSECUTIVE_DEVIATED_POINTS_FIELD_NAME, trackedJourney.longestConsecutiveDeviatedPoints); + Persistence.trackedJourneys.updateField( + trackedJourney.id, + TrackedJourney.LONGEST_CONSECUTIVE_DEVIATED_POINTS_FIELD_NAME, + trackedJourney.longestConsecutiveDeviatedPoints + ); // Provide response. return new EndTrackingResponse( From 2065188a6f2f6fb84961bbef67703d495100c8da Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 31 Oct 2024 14:53:27 -0400 Subject: [PATCH 31/39] style(NotificationUtils): Use static import --- .../opentripplanner/middleware/utils/NotificationUtils.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index 26409e74f..953704630 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -31,6 +31,7 @@ import java.util.stream.Collectors; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.opentripplanner.middleware.i18n.Message.TRIP_SURVEY_NOTIFICATION; import static org.opentripplanner.middleware.utils.ConfigUtils.getConfigPropertyAsText; /** @@ -101,10 +102,7 @@ public static String sendTripSurveyPush(OtpUser otpUser, MonitoredTrip trip) { Locale locale = I18nUtils.getOtpUserLocale(otpUser); String tripTime = DateTimeUtils.formatShortDate(trip.itinerary.startTime, locale); - String body = String.format( - org.opentripplanner.middleware.i18n.Message.TRIP_SURVEY_NOTIFICATION.get(locale), - tripTime - ); + String body = String.format(TRIP_SURVEY_NOTIFICATION.get(locale), tripTime); return sendPush(otpUser, body, trip.tripName, trip.id, TRIP_SURVEY_ID); } From de1d9f401be4a024d2c72852f659163d635fcf19 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 1 Nov 2024 16:23:26 -0400 Subject: [PATCH 32/39] docs: Introduce TRIP_SURVEY_SUBDOMAIN config param --- README.md | 1 + configurations/default/env.yml.tmp | 3 ++- src/main/resources/env.schema.json | 5 +++++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index ec5c1a3b3..6088bc75e 100644 --- a/README.md +++ b/README.md @@ -301,6 +301,7 @@ The special E2E client settings should be defined in `env.yml`: | TRIP_INSTRUCTION_IMMEDIATE_RADIUS | integer | Optional | 2 | The radius in meters under which an immediate instruction is given. | | TRIP_INSTRUCTION_UPCOMING_RADIUS | integer | Optional | 10 | The radius in meters under which an upcoming instruction is given. | | TRIP_SURVEY_ID | string | Optional | abcdef123y | The ID of a survey (on the platform of your choice) for trip-related feedback. | +| TRIP_SURVEY_SUBDOMAIN | string | Optional | abcabc12a | The subdomain of a website where the trip-related surveys are administered. | | TWILIO_ACCOUNT_SID | string | Optional | your-account-sid | Twilio settings available at: https://twilio.com/user/account | | TWILIO_AUTH_TOKEN | string | Optional | your-auth-token | Twilio settings available at: https://twilio.com/user/account | | US_RIDE_GWINNETT_BUS_OPERATOR_NOTIFIER_API_URL | string | Optional | http://host.example.com | US RideGwinnett bus notifier API. | diff --git a/configurations/default/env.yml.tmp b/configurations/default/env.yml.tmp index 32b691026..be93550a6 100644 --- a/configurations/default/env.yml.tmp +++ b/configurations/default/env.yml.tmp @@ -100,8 +100,9 @@ TRIP_INSTRUCTION_IMMEDIATE_RADIUS: 2 # The radius in meters under which an upcoming instruction is given. TRIP_INSTRUCTION_UPCOMING_RADIUS: 10 -# Survey ID that is linked to a particular trip. +# Survey ID and domain that is offered after users complete certain trips. TRIP_SURVEY_ID: abcdef123y +TRIP_SURVEY_SUBDOMAIN: abcabc12a US_RIDE_GWINNETT_BUS_OPERATOR_NOTIFIER_API_URL: https://bus.notifier.example.com US_RIDE_GWINNETT_BUS_OPERATOR_NOTIFIER_API_KEY: your-key diff --git a/src/main/resources/env.schema.json b/src/main/resources/env.schema.json index afa20b2a5..8a16c84aa 100644 --- a/src/main/resources/env.schema.json +++ b/src/main/resources/env.schema.json @@ -324,6 +324,11 @@ "examples": ["abcdef123y"], "description": "The ID of a survey (on the platform of your choice) for trip-related feedback." }, + "TRIP_SURVEY_SUBDOMAIN": { + "type": "string", + "examples": ["abcabc12a"], + "description": "The subdomain of a website where the trip-related surveys are administered." + }, "TWILIO_ACCOUNT_SID": { "type": "string", "examples": ["your-account-sid"], From 5bfe6538ef64f8f6e6f1a89186aa786e32f7d551 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 1 Nov 2024 16:35:50 -0400 Subject: [PATCH 33/39] feat(NotificationUtils): Include survey subdomain to push notification --- .../middleware/utils/NotificationUtils.java | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index 953704630..54a104ff2 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -10,6 +10,7 @@ import com.twilio.rest.verify.v2.service.VerificationCreator; import com.twilio.type.PhoneNumber; import freemarker.template.TemplateException; +import org.apache.logging.log4j.util.Strings; import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.util.StringUtil; import org.opentripplanner.middleware.bugsnag.BugsnagReporter; @@ -54,6 +55,7 @@ public class NotificationUtils { private static final String PUSH_API_KEY = getConfigPropertyAsText("PUSH_API_KEY"); private static final String PUSH_API_URL = getConfigPropertyAsText("PUSH_API_URL"); private static final String TRIP_SURVEY_ID = getConfigPropertyAsText("TRIP_SURVEY_ID"); + private static final String TRIP_SURVEY_SUBDOMAIN = getConfigPropertyAsText("TRIP_SURVEY_SUBDOMAIN"); /** * Although SMS are 160 characters long and Twilio supports sending up to 1600 characters, @@ -98,12 +100,19 @@ public static String sendTripSurveyPush(OtpUser otpUser, MonitoredTrip trip) { if (otpUser.pushDevices == 0) return "OK"; // If Push API/survey config properties aren't set, do nothing (will trigger warning log). - if (PUSH_API_KEY == null || PUSH_API_URL == null || TRIP_SURVEY_ID == null) return null; + if ( + Strings.isBlank(PUSH_API_KEY) || + Strings.isBlank(PUSH_API_URL) || + Strings.isBlank(TRIP_SURVEY_ID) || + Strings.isBlank(TRIP_SURVEY_SUBDOMAIN) + ) { + return null; + } Locale locale = I18nUtils.getOtpUserLocale(otpUser); String tripTime = DateTimeUtils.formatShortDate(trip.itinerary.startTime, locale); String body = String.format(TRIP_SURVEY_NOTIFICATION.get(locale), tripTime); - return sendPush(otpUser, body, trip.tripName, trip.id, TRIP_SURVEY_ID); + return sendPush(otpUser, body, trip.tripName, trip.id, TRIP_SURVEY_ID, TRIP_SURVEY_SUBDOMAIN); } /** @@ -115,14 +124,15 @@ public static String sendTripSurveyPush(OtpUser otpUser, MonitoredTrip trip) { * @param surveyId Survey ID * @return "OK" if message was successful (null otherwise) */ - static String sendPush(OtpUser toUser, String body, String tripName, String tripId, String surveyId) { + static String sendPush(OtpUser toUser, String body, String tripName, String tripId, String surveyId, String surveySubdomain) { try { NotificationInfo notificationInfo = new NotificationInfo( toUser, body, tripName, tripId, - surveyId + surveyId, + surveySubdomain ); var jsonBody = new Gson().toJson(notificationInfo); var httpResponse = HttpUtils.httpRequestRawResponse( @@ -435,11 +445,14 @@ static class NotificationInfo { /** The ID of the survey to be launched for said trip, if applicable. */ public final String surveyId; + /** The subdomain of the website where the survey is administered, if applicable. */ + public final String surveySubdomain; + public NotificationInfo(OtpUser user, String message, String title, String tripId) { - this(user, message, title, tripId, null); + this(user, message, title, tripId, null, null); } - public NotificationInfo(OtpUser user, String message, String title, String tripId, String surveyId) { + public NotificationInfo(OtpUser user, String message, String title, String tripId, String surveyId, String surveySubdomain) { String truncatedTitle = StringUtil.truncate(title, PUSH_TITLE_MAX_LENGTH); int truncatedMessageLength = PUSH_TOTAL_MAX_LENGTH - truncatedTitle.length(); @@ -449,6 +462,7 @@ public NotificationInfo(OtpUser user, String message, String title, String tripI this.message = StringUtil.truncate(message, truncatedMessageLength); this.tripId = tripId; this.surveyId = surveyId; + this.surveySubdomain = surveySubdomain; } } } From e8256da2029cadde339ef4c74f8f5e54fba9d609 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 5 Nov 2024 16:18:06 -0500 Subject: [PATCH 34/39] refactor(OtpUser): Use list of sent notifications for traceability. --- .../middleware/models/OtpUser.java | 12 ++++++-- .../models/TripSurveyNotification.java | 28 +++++++++++++++++++ .../triptracker/TripSurveySenderJob.java | 19 ++++++++++--- .../triptracker/TripSurveySenderJobTest.java | 24 +++++++++------- 4 files changed, 66 insertions(+), 17 deletions(-) create mode 100644 src/main/java/org/opentripplanner/middleware/models/TripSurveyNotification.java diff --git a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java index 1dbc1d476..b00e1ef85 100644 --- a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java @@ -26,7 +26,7 @@ public enum Notification { public static final String AUTH0_SCOPE = "otp-user"; private static final long serialVersionUID = 1L; private static final Logger LOG = LoggerFactory.getLogger(OtpUser.class); - public static final String LAST_TRIP_SURVEY_NOTIF_SENT_FIELD = "lastTripSurveyNotificationSent"; + public static final String TRIP_SURVEY_NOTIFICATIONS_FIELD = "tripSurveyNotifications"; /** Whether the user would like accessible routes by default. */ public boolean accessibilityRoutingByDefault; @@ -77,8 +77,8 @@ public enum Notification { /** Whether to store the user's trip history (user must opt in). */ public boolean storeTripHistory; - /** When the last post-trip survey notification was sent. */ - public Date lastTripSurveyNotificationSent; + /** The trail of survey notifications sent for journeys completed by the user. */ + public List tripSurveyNotifications = new ArrayList<>(); @JsonIgnore /** If this user was created by an {@link ApiUser}, this parameter will match the {@link ApiUser}'s id */ @@ -164,4 +164,10 @@ public void setNotificationChannel(String channels) { }); } } + + /** Obtains the last trip survey notification sent. */ + public Optional findLastTripSurveyNotificationSent() { + if (tripSurveyNotifications == null) return Optional.empty(); + return tripSurveyNotifications.stream().max(Comparator.comparingLong(n -> n.timeSent.getTime())); + } } diff --git a/src/main/java/org/opentripplanner/middleware/models/TripSurveyNotification.java b/src/main/java/org/opentripplanner/middleware/models/TripSurveyNotification.java new file mode 100644 index 000000000..c453fc83e --- /dev/null +++ b/src/main/java/org/opentripplanner/middleware/models/TripSurveyNotification.java @@ -0,0 +1,28 @@ +package org.opentripplanner.middleware.models; + +import java.util.Date; +import java.util.UUID; + +/** Contains information regarding survey notifications sent after a trip is completed. */ +public class TripSurveyNotification { + /** + * Unique ID to link a survey entry to the corresponding notification + * (and to find which notifications were dismissed without opening the survey) + */ + public String id = UUID.randomUUID().toString(); + + /** Date/time when the trip survey notification was sent. */ + public Date timeSent; + + /** The {@link TrackedJourney} (and, indirectly, the {@link MonitoredTrip}) that this notification refers to. */ + public String journeyId; + + public TripSurveyNotification() { + // Default constructor for deserialization + } + + public TripSurveyNotification(Date timeSent, String journeyId) { + this.timeSent = timeSent; + this.journeyId = journeyId; + } +} diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java index 5605a9081..6c47168ee 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java @@ -1,10 +1,12 @@ package org.opentripplanner.middleware.triptracker; import com.mongodb.client.model.Filters; +import org.bson.Document; import org.bson.conversions.Bson; import org.opentripplanner.middleware.models.MonitoredTrip; import org.opentripplanner.middleware.models.OtpUser; import org.opentripplanner.middleware.models.TrackedJourney; +import org.opentripplanner.middleware.models.TripSurveyNotification; import org.opentripplanner.middleware.persistence.Persistence; import org.opentripplanner.middleware.utils.NotificationUtils; import org.slf4j.Logger; @@ -25,7 +27,7 @@ import static org.opentripplanner.middleware.controllers.api.ApiController.ID_FIELD_NAME; import static org.opentripplanner.middleware.models.MonitoredTrip.USER_ID_FIELD_NAME; -import static org.opentripplanner.middleware.models.OtpUser.LAST_TRIP_SURVEY_NOTIF_SENT_FIELD; +import static org.opentripplanner.middleware.models.OtpUser.TRIP_SURVEY_NOTIFICATIONS_FIELD; import static org.opentripplanner.middleware.models.TrackedJourney.END_CONDITION_FIELD_NAME; import static org.opentripplanner.middleware.models.TrackedJourney.END_TIME_FIELD_NAME; import static org.opentripplanner.middleware.models.TrackedJourney.FORCIBLY_TERMINATED; @@ -63,7 +65,8 @@ public void run() { String pushResult = NotificationUtils.sendTripSurveyPush(otpUser, trip); if (pushResult != null) { // Store time of last sent survey notification for user. - Persistence.otpUsers.updateField(otpUser.id, LAST_TRIP_SURVEY_NOTIF_SENT_FIELD, new Date()); + otpUser.tripSurveyNotifications.add(new TripSurveyNotification(new Date(), optJourney.get().id)); + Persistence.otpUsers.updateField(otpUser.id, TRIP_SURVEY_NOTIFICATIONS_FIELD, otpUser.tripSurveyNotifications); } else { LOG.warn("Could not send survey notification for trip {}", trip.id); } @@ -79,8 +82,16 @@ public void run() { */ public static List getUsersWithNotificationsOverAWeekAgo() { Date aWeekAgo = Date.from(Instant.now().minus(7, ChronoUnit.DAYS)); - Bson dateFilter = Filters.lte(LAST_TRIP_SURVEY_NOTIF_SENT_FIELD, aWeekAgo); - Bson surveyNotSentFilter = Filters.not(Filters.exists(LAST_TRIP_SURVEY_NOTIF_SENT_FIELD)); + Bson dateFilter = Filters.all( + TRIP_SURVEY_NOTIFICATIONS_FIELD, + // Filters.elemMatch doesn't work well when all elements must match the timeSent filter. + new Document("$elemMatch", Filters.lte("timeSent", aWeekAgo)) + ); + + Bson surveyNotSentFilter = Filters.or( + Filters.not(Filters.exists(TRIP_SURVEY_NOTIFICATIONS_FIELD)), + Filters.size(TRIP_SURVEY_NOTIFICATIONS_FIELD, 0) + ); Bson overallFilter = Filters.or(dateFilter, surveyNotSentFilter); return Persistence.otpUsers.getFiltered(overallFilter).into(new ArrayList<>()); diff --git a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java index 8e3e4234a..f244a0096 100644 --- a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java +++ b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java @@ -7,6 +7,7 @@ import org.opentripplanner.middleware.models.MonitoredTrip; import org.opentripplanner.middleware.models.OtpUser; import org.opentripplanner.middleware.models.TrackedJourney; +import org.opentripplanner.middleware.models.TripSurveyNotification; import org.opentripplanner.middleware.otp.response.Itinerary; import org.opentripplanner.middleware.persistence.Persistence; import org.opentripplanner.middleware.testutils.ApiTestUtils; @@ -22,10 +23,9 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; -import static org.opentripplanner.middleware.models.OtpUser.LAST_TRIP_SURVEY_NOTIF_SENT_FIELD; +import static org.opentripplanner.middleware.models.OtpUser.TRIP_SURVEY_NOTIFICATIONS_FIELD; import static org.opentripplanner.middleware.models.TrackedJourney.FORCIBLY_TERMINATED; import static org.opentripplanner.middleware.models.TrackedJourney.TERMINATED_BY_USER; @@ -39,6 +39,7 @@ class TripSurveySenderJobTest extends OtpMiddlewareTestEnvironment { private static List journeys = List.of(); private static MonitoredTrip trip; private static final Date EIGHT_DAYS_AGO = Date.from(Instant.now().minus(8, ChronoUnit.DAYS)); + private static final TripSurveyNotification SURVEY_NOTIFICATION_EIGHT_DAYS_AGO = new TripSurveyNotification(EIGHT_DAYS_AGO, "journey-2"); @BeforeAll public static void setUp() { @@ -49,9 +50,8 @@ public static void setUp() { user2notifiedAWeekAgo = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("test-user2")); user3neverNotified = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("test-user3")); - user1notifiedNow.lastTripSurveyNotificationSent = new Date(); - user2notifiedAWeekAgo.lastTripSurveyNotificationSent = EIGHT_DAYS_AGO; - user3neverNotified.lastTripSurveyNotificationSent = null; + user1notifiedNow.tripSurveyNotifications.add(new TripSurveyNotification(new Date(), "journey-1")); + user2notifiedAWeekAgo.tripSurveyNotifications.add(SURVEY_NOTIFICATION_EIGHT_DAYS_AGO); otpUsers = List.of(user1notifiedNow, user2notifiedAWeekAgo, user3neverNotified); otpUsers.forEach(user -> Persistence.otpUsers.replace(user.id, user)); @@ -87,7 +87,11 @@ void afterEach() { } // Reset notification sent time to affected user. - Persistence.otpUsers.updateField(user2notifiedAWeekAgo.id, LAST_TRIP_SURVEY_NOTIF_SENT_FIELD, EIGHT_DAYS_AGO); + Persistence.otpUsers.updateField( + user2notifiedAWeekAgo.id, + TRIP_SURVEY_NOTIFICATIONS_FIELD, + List.of(SURVEY_NOTIFICATION_EIGHT_DAYS_AGO) + ); } @Test @@ -183,19 +187,19 @@ void canRunJob() { Date start = new Date(); OtpUser storedUser = Persistence.otpUsers.getById(user2notifiedAWeekAgo.id); - assertFalse(start.before(storedUser.lastTripSurveyNotificationSent)); + assertFalse(start.before(storedUser.findLastTripSurveyNotificationSent().get().timeSent)); TripSurveySenderJob job = new TripSurveySenderJob(); job.run(); storedUser = Persistence.otpUsers.getById(user2notifiedAWeekAgo.id); - assertTrue(start.before(storedUser.lastTripSurveyNotificationSent)); + assertTrue(start.before(storedUser.findLastTripSurveyNotificationSent().get().timeSent)); // Other user last notification should not have changed. storedUser = Persistence.otpUsers.getById(user1notifiedNow.id); - assertFalse(start.before(storedUser.lastTripSurveyNotificationSent)); + assertFalse(start.before(storedUser.findLastTripSurveyNotificationSent().get().timeSent)); storedUser = Persistence.otpUsers.getById(user3neverNotified.id); - assertNull(storedUser.lastTripSurveyNotificationSent); + assertTrue(storedUser.findLastTripSurveyNotificationSent().isEmpty()); } private static void createTestJourneys() { From e096f849f02ea6dc890a2617e73b63c86dd0aa8f Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 5 Nov 2024 16:53:37 -0500 Subject: [PATCH 35/39] refactor(TripSurveySenderJob): Support instances where users had previous notifications=. --- .../triptracker/TripSurveySenderJob.java | 19 +++++++++++++++---- .../triptracker/TripSurveySenderJobTest.java | 4 ++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java index 6c47168ee..7e59eda45 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java @@ -82,10 +82,21 @@ public void run() { */ public static List getUsersWithNotificationsOverAWeekAgo() { Date aWeekAgo = Date.from(Instant.now().minus(7, ChronoUnit.DAYS)); - Bson dateFilter = Filters.all( - TRIP_SURVEY_NOTIFICATIONS_FIELD, - // Filters.elemMatch doesn't work well when all elements must match the timeSent filter. - new Document("$elemMatch", Filters.lte("timeSent", aWeekAgo)) + + // If TRIP_SURVEY_NOTIFICATIONS_FIELD is not empty, + // users notified a week ago would have at least one entry made a week ago + // and zero entries made less than a week ago. + Bson dateFilter = Filters.and( + Filters.all( + TRIP_SURVEY_NOTIFICATIONS_FIELD, + new Document("$elemMatch", Filters.lte("timeSent", aWeekAgo)) + ), + Filters.not( + Filters.all( + TRIP_SURVEY_NOTIFICATIONS_FIELD, + new Document("$elemMatch", Filters.gt("timeSent", aWeekAgo)) + ) + ) ); Bson surveyNotSentFilter = Filters.or( diff --git a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java index f244a0096..53c52baaa 100644 --- a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java +++ b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java @@ -51,7 +51,11 @@ public static void setUp() { user3neverNotified = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("test-user3")); user1notifiedNow.tripSurveyNotifications.add(new TripSurveyNotification(new Date(), "journey-1")); + user1notifiedNow.tripSurveyNotifications.add(SURVEY_NOTIFICATION_EIGHT_DAYS_AGO); user2notifiedAWeekAgo.tripSurveyNotifications.add(SURVEY_NOTIFICATION_EIGHT_DAYS_AGO); + user2notifiedAWeekAgo.tripSurveyNotifications.add( + new TripSurveyNotification(Date.from(Instant.EPOCH), "journey-1") + ); otpUsers = List.of(user1notifiedNow, user2notifiedAWeekAgo, user3neverNotified); otpUsers.forEach(user -> Persistence.otpUsers.replace(user.id, user)); From 402383ff42e1589c8da5cc225fbd194189bbe088 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 5 Nov 2024 17:03:50 -0500 Subject: [PATCH 36/39] refactor(TripSurveySenderJob): Improve Mongo filters. --- .../models/TripSurveyNotification.java | 3 +++ .../triptracker/TripSurveySenderJob.java | 20 ++++++------------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/models/TripSurveyNotification.java b/src/main/java/org/opentripplanner/middleware/models/TripSurveyNotification.java index c453fc83e..3daff367d 100644 --- a/src/main/java/org/opentripplanner/middleware/models/TripSurveyNotification.java +++ b/src/main/java/org/opentripplanner/middleware/models/TripSurveyNotification.java @@ -5,6 +5,9 @@ /** Contains information regarding survey notifications sent after a trip is completed. */ public class TripSurveyNotification { + + public static final String TIME_SENT_FIELD = "timeSent"; + /** * Unique ID to link a survey entry to the corresponding notification * (and to find which notifications were dismissed without opening the survey) diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java index 7e59eda45..fc7a9933f 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java @@ -1,7 +1,6 @@ package org.opentripplanner.middleware.triptracker; import com.mongodb.client.model.Filters; -import org.bson.Document; import org.bson.conversions.Bson; import org.opentripplanner.middleware.models.MonitoredTrip; import org.opentripplanner.middleware.models.OtpUser; @@ -32,6 +31,7 @@ import static org.opentripplanner.middleware.models.TrackedJourney.END_TIME_FIELD_NAME; import static org.opentripplanner.middleware.models.TrackedJourney.FORCIBLY_TERMINATED; import static org.opentripplanner.middleware.models.TrackedJourney.TERMINATED_BY_USER; +import static org.opentripplanner.middleware.models.TripSurveyNotification.TIME_SENT_FIELD; import static org.opentripplanner.middleware.triptracker.ManageTripTracking.TRIP_TRACKING_UPDATE_FREQUENCY_SECONDS; /** @@ -83,20 +83,12 @@ public void run() { public static List getUsersWithNotificationsOverAWeekAgo() { Date aWeekAgo = Date.from(Instant.now().minus(7, ChronoUnit.DAYS)); - // If TRIP_SURVEY_NOTIFICATIONS_FIELD is not empty, - // users notified a week ago would have at least one entry made a week ago - // and zero entries made less than a week ago. + // If TRIP_SURVEY_NOTIFICATIONS_FIELD is not empty, users notified a week ago would have: + // - at least one entry made a week ago, and + // - zero entries made less than a week ago. Bson dateFilter = Filters.and( - Filters.all( - TRIP_SURVEY_NOTIFICATIONS_FIELD, - new Document("$elemMatch", Filters.lte("timeSent", aWeekAgo)) - ), - Filters.not( - Filters.all( - TRIP_SURVEY_NOTIFICATIONS_FIELD, - new Document("$elemMatch", Filters.gt("timeSent", aWeekAgo)) - ) - ) + Filters.elemMatch(TRIP_SURVEY_NOTIFICATIONS_FIELD, Filters.lte(TIME_SENT_FIELD, aWeekAgo)), + Filters.not(Filters.elemMatch(TRIP_SURVEY_NOTIFICATIONS_FIELD, Filters.gt(TIME_SENT_FIELD, aWeekAgo))) ); Bson surveyNotSentFilter = Filters.or( From 986d5ce4491f080c5e452d1022de80e395fca9f8 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 5 Nov 2024 21:05:54 -0500 Subject: [PATCH 37/39] refactor(TripSurveySenderJob): Include notification id. --- .../models/TripSurveyNotification.java | 5 ++- .../triptracker/TripSurveySenderJob.java | 6 ++- .../middleware/utils/NotificationUtils.java | 37 +++++++++++++++---- .../triptracker/TripSurveySenderJobTest.java | 17 +++++++-- .../utils/NotificationUtilsTest.java | 1 + 5 files changed, 51 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/models/TripSurveyNotification.java b/src/main/java/org/opentripplanner/middleware/models/TripSurveyNotification.java index 3daff367d..0d6ec8af0 100644 --- a/src/main/java/org/opentripplanner/middleware/models/TripSurveyNotification.java +++ b/src/main/java/org/opentripplanner/middleware/models/TripSurveyNotification.java @@ -12,7 +12,7 @@ public class TripSurveyNotification { * Unique ID to link a survey entry to the corresponding notification * (and to find which notifications were dismissed without opening the survey) */ - public String id = UUID.randomUUID().toString(); + public String id; /** Date/time when the trip survey notification was sent. */ public Date timeSent; @@ -24,7 +24,8 @@ public TripSurveyNotification() { // Default constructor for deserialization } - public TripSurveyNotification(Date timeSent, String journeyId) { + public TripSurveyNotification(String id, Date timeSent, String journeyId) { + this.id = id; this.timeSent = timeSent; this.journeyId = journeyId; } diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java index fc7a9933f..f82d65081 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJob.java @@ -21,6 +21,7 @@ import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.UUID; import java.util.function.Function; import java.util.stream.Collectors; @@ -62,10 +63,11 @@ public void run() { MonitoredTrip trip = optJourney.get().trip; LOG.info("Sending survey notification for trip {}", trip.id); OtpUser otpUser = entry.getKey(); - String pushResult = NotificationUtils.sendTripSurveyPush(otpUser, trip); + String notificationId = UUID.randomUUID().toString(); + String pushResult = NotificationUtils.sendTripSurveyPush(otpUser, trip, notificationId); if (pushResult != null) { // Store time of last sent survey notification for user. - otpUser.tripSurveyNotifications.add(new TripSurveyNotification(new Date(), optJourney.get().id)); + otpUser.tripSurveyNotifications.add(new TripSurveyNotification(notificationId, new Date(), optJourney.get().id)); Persistence.otpUsers.updateField(otpUser.id, TRIP_SURVEY_NOTIFICATIONS_FIELD, otpUser.tripSurveyNotifications); } else { LOG.warn("Could not send survey notification for trip {}", trip.id); diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index 54a104ff2..df51d635e 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -83,7 +83,7 @@ public static String sendPush(OtpUser otpUser, String textTemplate, Object templ if (PUSH_API_KEY == null || PUSH_API_URL == null) return null; try { String body = TemplateUtils.renderTemplate(textTemplate, templateData); - return otpUser.pushDevices > 0 ? sendPush(otpUser, body, tripName, tripId, null) : "OK"; + return otpUser.pushDevices > 0 ? sendPush(otpUser, body, tripName, tripId, null, null, null) : "OK"; } catch (TemplateException | IOException e) { // This catch indicates there was an error rendering the template. Note: TemplateUtils#renderTemplate // handles Bugsnag reporting/error logging, so that is not needed here. @@ -94,8 +94,9 @@ public static String sendPush(OtpUser otpUser, String textTemplate, Object templ /** * @param otpUser target user * @param trip Trip about which the survey notification is about. + * @param notificationId Notification ID */ - public static String sendTripSurveyPush(OtpUser otpUser, MonitoredTrip trip) { + public static String sendTripSurveyPush(OtpUser otpUser, MonitoredTrip trip, String notificationId) { // Check devices first - No devices returns OK (favors E2E testing) if (otpUser.pushDevices == 0) return "OK"; @@ -112,7 +113,7 @@ public static String sendTripSurveyPush(OtpUser otpUser, MonitoredTrip trip) { Locale locale = I18nUtils.getOtpUserLocale(otpUser); String tripTime = DateTimeUtils.formatShortDate(trip.itinerary.startTime, locale); String body = String.format(TRIP_SURVEY_NOTIFICATION.get(locale), tripTime); - return sendPush(otpUser, body, trip.tripName, trip.id, TRIP_SURVEY_ID, TRIP_SURVEY_SUBDOMAIN); + return sendPush(otpUser, body, trip.tripName, trip.id, TRIP_SURVEY_ID, TRIP_SURVEY_SUBDOMAIN, notificationId); } /** @@ -122,11 +123,21 @@ public static String sendTripSurveyPush(OtpUser otpUser, MonitoredTrip trip) { * @param tripName Monitored trip name to show in notification title * @param tripId Monitored trip ID * @param surveyId Survey ID + * @param notificationId Notification ID * @return "OK" if message was successful (null otherwise) */ - static String sendPush(OtpUser toUser, String body, String tripName, String tripId, String surveyId, String surveySubdomain) { + static String sendPush( + OtpUser toUser, + String body, + String tripName, + String tripId, + String surveyId, + String surveySubdomain, + String notificationId + ) { try { NotificationInfo notificationInfo = new NotificationInfo( + notificationId, toUser, body, tripName, @@ -427,6 +438,9 @@ public static void updatePushDevices(OtpUser otpUser) { } static class NotificationInfo { + /** ID for tracking notifications and survey responses. */ + public final String notificationId; + /** In reality, the email of the desired user (the push service we use looks up users by email) */ public final String user; @@ -448,14 +462,23 @@ static class NotificationInfo { /** The subdomain of the website where the survey is administered, if applicable. */ public final String surveySubdomain; - public NotificationInfo(OtpUser user, String message, String title, String tripId) { - this(user, message, title, tripId, null, null); + public NotificationInfo(String notificationId, OtpUser user, String message, String title, String tripId) { + this(notificationId, user, message, title, tripId, null, null); } - public NotificationInfo(OtpUser user, String message, String title, String tripId, String surveyId, String surveySubdomain) { + public NotificationInfo( + String notificationId, + OtpUser user, + String message, + String title, + String tripId, + String surveyId, + String surveySubdomain + ) { String truncatedTitle = StringUtil.truncate(title, PUSH_TITLE_MAX_LENGTH); int truncatedMessageLength = PUSH_TOTAL_MAX_LENGTH - truncatedTitle.length(); + this.notificationId = notificationId; this.user = user.email; this.userId = user.id; this.title = truncatedTitle; diff --git a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java index 53c52baaa..382efe4e1 100644 --- a/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java +++ b/src/test/java/org/opentripplanner/middleware/triptracker/TripSurveySenderJobTest.java @@ -23,6 +23,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; import static org.opentripplanner.middleware.models.OtpUser.TRIP_SURVEY_NOTIFICATIONS_FIELD; @@ -39,7 +40,11 @@ class TripSurveySenderJobTest extends OtpMiddlewareTestEnvironment { private static List journeys = List.of(); private static MonitoredTrip trip; private static final Date EIGHT_DAYS_AGO = Date.from(Instant.now().minus(8, ChronoUnit.DAYS)); - private static final TripSurveyNotification SURVEY_NOTIFICATION_EIGHT_DAYS_AGO = new TripSurveyNotification(EIGHT_DAYS_AGO, "journey-2"); + private static final TripSurveyNotification SURVEY_NOTIFICATION_EIGHT_DAYS_AGO = new TripSurveyNotification( + "notification-8-days-ago", + EIGHT_DAYS_AGO, + "journey-2" + ); @BeforeAll public static void setUp() { @@ -50,11 +55,13 @@ public static void setUp() { user2notifiedAWeekAgo = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("test-user2")); user3neverNotified = PersistenceTestUtils.createUser(ApiTestUtils.generateEmailAddress("test-user3")); - user1notifiedNow.tripSurveyNotifications.add(new TripSurveyNotification(new Date(), "journey-1")); + user1notifiedNow.tripSurveyNotifications.add( + new TripSurveyNotification("notification-id-1", new Date(), "journey-1") + ); user1notifiedNow.tripSurveyNotifications.add(SURVEY_NOTIFICATION_EIGHT_DAYS_AGO); user2notifiedAWeekAgo.tripSurveyNotifications.add(SURVEY_NOTIFICATION_EIGHT_DAYS_AGO); user2notifiedAWeekAgo.tripSurveyNotifications.add( - new TripSurveyNotification(Date.from(Instant.EPOCH), "journey-1") + new TripSurveyNotification("notification-id-2", Date.from(Instant.EPOCH), "journey-1") ); otpUsers = List.of(user1notifiedNow, user2notifiedAWeekAgo, user3neverNotified); @@ -201,7 +208,9 @@ void canRunJob() { // Other user last notification should not have changed. storedUser = Persistence.otpUsers.getById(user1notifiedNow.id); - assertFalse(start.before(storedUser.findLastTripSurveyNotificationSent().get().timeSent)); + Optional notification = storedUser.findLastTripSurveyNotificationSent(); + assertFalse(start.before(notification.get().timeSent)); + assertNotNull(notification.get().id); storedUser = Persistence.otpUsers.getById(user3neverNotified.id); assertTrue(storedUser.findLastTripSurveyNotificationSent().isEmpty()); } diff --git a/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java b/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java index 1df4f7c56..22f6b546e 100644 --- a/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java +++ b/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java @@ -91,6 +91,7 @@ void testTruncateNotificationPayload(String originalTitle, String expectedTitle, user.email = "user@example.com"; NotificationUtils.NotificationInfo info = new NotificationUtils.NotificationInfo( + "notification-id", user, originalMessage, originalTitle, From 9f2ab383021460ec0b63ae90e971b38d492641d5 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 6 Nov 2024 08:09:51 -0500 Subject: [PATCH 38/39] refactor(OtpUser): Fix imports --- .../java/org/opentripplanner/middleware/models/OtpUser.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java index c1dcc06af..c42d90f6d 100644 --- a/src/main/java/org/opentripplanner/middleware/models/OtpUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/OtpUser.java @@ -11,10 +11,12 @@ import java.util.ArrayList; +import java.util.Comparator; import java.util.Date; import java.util.EnumSet; import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.stream.Collectors; import java.util.stream.Stream; From bb99963ee079eb199ebef7440e6b04f77fb50e12 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 6 Nov 2024 08:16:08 -0500 Subject: [PATCH 39/39] docs(Swagger): Update snapshots --- .../latest-spark-swagger-output.yaml | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/main/resources/latest-spark-swagger-output.yaml b/src/main/resources/latest-spark-swagger-output.yaml index f9a9f8302..ab497fc4b 100644 --- a/src/main/resources/latest-spark-swagger-output.yaml +++ b/src/main/resources/latest-spark-swagger-output.yaml @@ -1651,10 +1651,10 @@ paths: "200": description: "Successful operation" examples: {} - schema: - $ref: "#/definitions/OtpUser" responseSchema: $ref: "#/definitions/OtpUser" + schema: + $ref: "#/definitions/OtpUser" "400": description: "The request was not formed properly (e.g., some required parameters\ \ may be missing). See the details of the returned response to determine\ @@ -3173,9 +3173,10 @@ definitions: $ref: "#/definitions/UserLocation" storeTripHistory: type: "boolean" - lastTripSurveyNotificationSent: - type: "string" - format: "date" + tripSurveyNotifications: + type: "array" + items: + $ref: "#/definitions/TripSurveyNotification" applicationId: type: "string" relatedUsers: @@ -3205,6 +3206,16 @@ definitions: - "LEGALLY_BLIND" - "LOW_VISION" - "NONE" + TripSurveyNotification: + type: "object" + properties: + id: + type: "string" + timeSent: + type: "string" + format: "date" + journeyId: + type: "string" GetUsageResult: type: "object" properties: