From aab57b57885aabefa0b0e9dadc7c02659c0e45e2 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Wed, 13 Nov 2024 13:57:21 +0000 Subject: [PATCH 01/26] refactor(Initial work to provide traveler with continue instruction): --- pom.xml | 7 +++ .../triptracker/TravelerLocator.java | 51 ++++++++++++++++-- .../instruction/ContinueInstruction.java | 23 ++++++++ .../api/TrackedTripControllerTest.java | 54 +++++++++++++------ .../triptracker/ManageLegTraversalTest.java | 9 ++-- 5 files changed, 121 insertions(+), 23 deletions(-) create mode 100644 src/main/java/org/opentripplanner/middleware/triptracker/instruction/ContinueInstruction.java diff --git a/pom.xml b/pom.xml index d0131965f..26fce5f3e 100644 --- a/pom.xml +++ b/pom.xml @@ -125,6 +125,13 @@ 2.7 + + + org.apache.commons + commons-lang3 + 3.17.0 + + org.slf4j diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TravelerLocator.java b/src/main/java/org/opentripplanner/middleware/triptracker/TravelerLocator.java index e444fbe62..ce2fa62fe 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/TravelerLocator.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/TravelerLocator.java @@ -4,6 +4,7 @@ import org.opentripplanner.middleware.otp.response.Leg; import org.opentripplanner.middleware.otp.response.Place; import org.opentripplanner.middleware.otp.response.Step; +import org.opentripplanner.middleware.triptracker.instruction.ContinueInstruction; import org.opentripplanner.middleware.triptracker.instruction.DeviatedInstruction; import org.opentripplanner.middleware.triptracker.instruction.GetOffHereTransitInstruction; import org.opentripplanner.middleware.triptracker.instruction.GetOffNextStopTransitInstruction; @@ -27,7 +28,9 @@ import java.util.Map; import java.util.Optional; import java.util.stream.Collectors; +import java.util.stream.IntStream; +import static org.apache.commons.lang3.ObjectUtils.isNotEmpty; import static org.opentripplanner.middleware.triptracker.instruction.TripInstruction.NO_INSTRUCTION; import static org.opentripplanner.middleware.triptracker.instruction.TripInstruction.TRIP_INSTRUCTION_IMMEDIATE_RADIUS; import static org.opentripplanner.middleware.triptracker.instruction.TripInstruction.TRIP_INSTRUCTION_UPCOMING_RADIUS; @@ -59,7 +62,7 @@ public static String getInstruction( ) { if (hasRequiredWalkLeg(travelerPosition)) { if (hasRequiredTripStatus(tripStatus)) { - TripInstruction tripInstruction = alignTravelerToTrip(travelerPosition, isStartOfTrip); + TripInstruction tripInstruction = alignTravelerToTrip(travelerPosition, isStartOfTrip, false); if (tripInstruction != null) { return tripInstruction.build(); } @@ -124,7 +127,7 @@ private static TripInstruction getBackOnTrack( TravelerPosition travelerPosition, boolean isStartOfTrip ) { - TripInstruction instruction = alignTravelerToTrip(travelerPosition, isStartOfTrip); + TripInstruction instruction = alignTravelerToTrip(travelerPosition, isStartOfTrip, true); if (instruction != null && instruction.hasInstruction()) { return instruction; } @@ -169,7 +172,8 @@ private static String getBusStopName(Leg busLeg) { @Nullable public static TripInstruction alignTravelerToTrip( TravelerPosition travelerPosition, - boolean isStartOfTrip + boolean isStartOfTrip, + boolean travelerHasDeviated ) { Locale locale = travelerPosition.locale; @@ -182,16 +186,55 @@ public static TripInstruction alignTravelerToTrip( } Step nextStep = snapToWaypoint(travelerPosition, travelerPosition.expectedLeg.steps); + TripInstruction tripInstruction = null; if (nextStep != null && (!isPositionPastStep(travelerPosition, nextStep) || isStartOfTrip)) { - return new OnTrackInstruction( + tripInstruction = new OnTrackInstruction( getDistance(travelerPosition.currentPosition, new Coordinates(nextStep)), nextStep, locale ); } + return (travelerHasDeviated || (isNotEmpty(tripInstruction) && tripInstruction.hasInstruction())) + ? tripInstruction + : getContinueInstruction(travelerPosition, nextStep, locale); + } + + /** + * Traveler is on track, but no immediate instruction is available. Provide a "continue on street" reassurance + * instruction providing they are on a walk leg. This will be based on the current or previous step depending on the + * traveler's relative position to the next leg. + */ + private static ContinueInstruction getContinueInstruction( + TravelerPosition travelerPosition, + Step nextStep, + Locale locale + ) { + if (!travelerPosition.expectedLeg.transitLeg && nextStep != null) { + Step currentStep = isPositionPastStep(travelerPosition, nextStep) + ? nextStep : + getPreviousStep(travelerPosition.expectedLeg.steps, nextStep); + if (currentStep != null) { + return new ContinueInstruction(currentStep, locale); + } + } return null; } + /** + * Get the step prior to the next step provided. + */ + private static Step getPreviousStep(List steps, Step nextStep) { + if (steps.get(0).equals(nextStep)) { + return null; + } + Optional previousStep = IntStream + .range(0, steps.size()) + .filter(i -> steps.get(i).equals(nextStep)) + .mapToObj(i -> steps.get(i - 1)) + .findFirst(); + return previousStep.orElse(null); + } + /** * Send bus notification if the first leg is a bus leg or approaching a bus leg and within the notify window. */ diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/instruction/ContinueInstruction.java b/src/main/java/org/opentripplanner/middleware/triptracker/instruction/ContinueInstruction.java new file mode 100644 index 000000000..c0f6f6915 --- /dev/null +++ b/src/main/java/org/opentripplanner/middleware/triptracker/instruction/ContinueInstruction.java @@ -0,0 +1,23 @@ +package org.opentripplanner.middleware.triptracker.instruction; + +import org.opentripplanner.middleware.otp.response.Step; + +import java.util.Locale; + +import static org.apache.commons.lang3.ObjectUtils.isNotEmpty; + +public class ContinueInstruction extends SelfLegInstruction { + public ContinueInstruction(Step legStep, Locale locale) { + this.legStep = legStep; + this.locale = locale; + } + + @Override + public String build() { + if (isNotEmpty(legStep) && isNotEmpty(legStep.streetName)) { + // TODO: i18n + return String.format("Continue on %s", legStep.streetName); + } + return NO_INSTRUCTION; + } +} 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 0d6d95bcc..0ec06df67 100644 --- a/src/test/java/org/opentripplanner/middleware/controllers/api/TrackedTripControllerTest.java +++ b/src/test/java/org/opentripplanner/middleware/controllers/api/TrackedTripControllerTest.java @@ -16,6 +16,7 @@ import org.opentripplanner.middleware.models.TrackedJourney; import org.opentripplanner.middleware.otp.response.Itinerary; import org.opentripplanner.middleware.otp.response.Leg; +import org.opentripplanner.middleware.otp.response.Step; import org.opentripplanner.middleware.persistence.Persistence; import org.opentripplanner.middleware.testutils.ApiTestUtils; import org.opentripplanner.middleware.testutils.CommonTestUtils; @@ -27,6 +28,10 @@ import org.opentripplanner.middleware.triptracker.TrackingLocation; import org.opentripplanner.middleware.triptracker.TripStatus; import org.opentripplanner.middleware.triptracker.TripTrackingData; +import org.opentripplanner.middleware.triptracker.instruction.ContinueInstruction; +import org.opentripplanner.middleware.triptracker.instruction.DeviatedInstruction; +import org.opentripplanner.middleware.triptracker.instruction.OnTrackInstruction; +import org.opentripplanner.middleware.triptracker.instruction.WaitForTransitInstruction; import org.opentripplanner.middleware.triptracker.payload.EndTrackingPayload; import org.opentripplanner.middleware.triptracker.payload.ForceEndTrackingPayload; import org.opentripplanner.middleware.triptracker.payload.StartTrackingPayload; @@ -39,10 +44,12 @@ import org.opentripplanner.middleware.utils.HttpResponseValues; import org.opentripplanner.middleware.utils.JsonUtils; +import java.time.Duration; import java.time.Instant; import java.util.Date; import java.util.HashMap; import java.util.List; +import java.util.Locale; import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -288,88 +295,105 @@ private static Stream createInstructionAndStatusCases() { final int NORTH_WEST_BEARING = 315; final int NORTH_EAST_BEARING = 45; final int WEST_BEARING = 270; + final Locale locale = Locale.US; Leg firstLeg = itinerary.legs.get(0); - Coordinates firstStepCoords = new Coordinates(firstLeg.steps.get(0)); - Coordinates thirdStepCoords = new Coordinates(firstLeg.steps.get(2)); + Step adairAvenueNortheastStep = firstLeg.steps.get(0); + Step virginiaCircleNortheastStep = firstLeg.steps.get(1); + Step ponceDeLeonPlaceNortheastStep = firstLeg.steps.get(2); + Coordinates firstStepCoords = new Coordinates(adairAvenueNortheastStep); + Coordinates thirdStepCoords = new Coordinates(ponceDeLeonPlaceNortheastStep); Coordinates destinationCoords = new Coordinates(firstLeg.to); + String monroeDrDestinationName = firstLeg.to.name; Leg multiItinFirstLeg = multiLegItinerary.legs.get(0); - Coordinates multiItinFirstLegDestCoords = new Coordinates(multiItinFirstLeg.to); Leg multiItinLastLeg = multiLegItinerary.legs.get(multiLegItinerary.legs.size() - 1); + Leg multiItinBusLeg = multiLegItinerary.legs.get(multiLegItinerary.legs.size() - 2); + Coordinates multiItinFirstLegDestCoords = new Coordinates(multiItinFirstLeg.to); Coordinates multiItinLastLegDestCoords = new Coordinates(multiItinLastLeg.to); + String ansleyMallPetShopDestinationName = multiItinLastLeg.to.name; return Stream.of( Arguments.of( monitoredTrip, createPoint(firstStepCoords, 1, NORTH_EAST_BEARING), - "IMMEDIATE: Head WEST on Adair Avenue Northeast", + new OnTrackInstruction(1, adairAvenueNortheastStep, locale).build(), TripStatus.ON_SCHEDULE, "Coords near first step should produce relevant instruction" ), Arguments.of( monitoredTrip, createPoint(firstStepCoords, 4, NORTH_EAST_BEARING), - "UPCOMING: Head WEST on Adair Avenue Northeast", + new OnTrackInstruction(4, adairAvenueNortheastStep, locale).build(), TripStatus.DEVIATED, "Coords deviated but near first step should produce relevant instruction" ), Arguments.of( monitoredTrip, createPoint(firstStepCoords, 30, NORTH_EAST_BEARING), - "Head to Adair Avenue Northeast", + new DeviatedInstruction(adairAvenueNortheastStep.streetName, locale).build(), TripStatus.DEVIATED, "Deviated coords near first step should produce instruction to head to first step #1" ), Arguments.of( monitoredTrip, createPoint(firstStepCoords, 15, NORTH_WEST_BEARING), - "Head to Adair Avenue Northeast", + new DeviatedInstruction(adairAvenueNortheastStep.streetName, locale).build(), TripStatus.DEVIATED, "Deviated coords near first step should produce instruction to head to first step #2" ), Arguments.of( monitoredTrip, createPoint(firstStepCoords, 20, WEST_BEARING), - NO_INSTRUCTION, + new ContinueInstruction(virginiaCircleNortheastStep, locale).build(), TripStatus.ON_SCHEDULE, - "Coords along a step should produce no instruction" + "Coords along a step should produce a continue on street instruction" ), Arguments.of( monitoredTrip, thirdStepCoords, - "IMMEDIATE: LEFT on Ponce de Leon Place Northeast", + new OnTrackInstruction(0, ponceDeLeonPlaceNortheastStep, locale).build(), TripStatus.AHEAD_OF_SCHEDULE, "Coords near a not-first step should produce relevant instruction" ), Arguments.of( monitoredTrip, createPoint(thirdStepCoords, 30, NORTH_WEST_BEARING), - "Head to Ponce de Leon Place Northeast", + new DeviatedInstruction(ponceDeLeonPlaceNortheastStep.streetName, locale).build(), TripStatus.DEVIATED, "Deviated coords near a not-first step should produce instruction to head to step" ), Arguments.of( monitoredTrip, createPoint(destinationCoords, 1, NORTH_WEST_BEARING), - "ARRIVED: Monroe Dr NE at Cooledge Ave NE", + new OnTrackInstruction(2, monroeDrDestinationName, locale).build(), TripStatus.COMPLETED, "Instructions for destination coordinate" ), Arguments.of( multiLegMonitoredTrip, createPoint(multiItinFirstLegDestCoords, 1.5, WEST_BEARING), - // Time is in US Pacific time zone (instead of US Eastern) by configuration for other E2E tests. - "Wait 6 minutes for your bus, route 27, scheduled at 9:18 AM, on time", + new WaitForTransitInstruction( + multiItinBusLeg, + multiItinBusLeg.getScheduledStartTime().toInstant().minus(Duration.ofMinutes(6)), + locale) + .build(), TripStatus.AHEAD_OF_SCHEDULE, "Arriving ahead of schedule to a bus stop at the end of first leg." ), Arguments.of( multiLegMonitoredTrip, createPoint(multiItinLastLegDestCoords, 1, NORTH_WEST_BEARING), - "ARRIVED: Ansley Mall Pet Shop", + new OnTrackInstruction(1, ansleyMallPetShopDestinationName, locale).build(), TripStatus.COMPLETED, "Instructions for destination coordinate of multi-leg trip" + ), + Arguments.of( + monitoredTrip, + createPoint(thirdStepCoords, 1000, NORTH_WEST_BEARING), + NO_INSTRUCTION, + TripStatus.DEVIATED, + "Deviated significantly from nearest step should produce no instruction" ) ); } diff --git a/src/test/java/org/opentripplanner/middleware/triptracker/ManageLegTraversalTest.java b/src/test/java/org/opentripplanner/middleware/triptracker/ManageLegTraversalTest.java index ea6d6abc6..e3d919b4f 100644 --- a/src/test/java/org/opentripplanner/middleware/triptracker/ManageLegTraversalTest.java +++ b/src/test/java/org/opentripplanner/middleware/triptracker/ManageLegTraversalTest.java @@ -15,6 +15,7 @@ import org.opentripplanner.middleware.otp.response.Place; import org.opentripplanner.middleware.otp.response.Step; import org.opentripplanner.middleware.testutils.CommonTestUtils; +import org.opentripplanner.middleware.triptracker.instruction.ContinueInstruction; import org.opentripplanner.middleware.triptracker.instruction.DeviatedInstruction; import org.opentripplanner.middleware.triptracker.instruction.OnTrackInstruction; import org.opentripplanner.middleware.utils.ConfigUtils; @@ -270,9 +271,9 @@ private static Stream createTurnByTurnTrace() { walkLeg, new TraceData( createPoint(virginiaCircleNortheastCoords, 12, SOUTH_WEST_BEARING), - NO_INSTRUCTION, + new ContinueInstruction(ponceDeLeonPlaceNortheastStep, locale).build(), false, - "On track approaching second step, but not close enough for instruction." + "On track approaching second step, provide continue instruction." ) ), Arguments.of( @@ -336,9 +337,9 @@ private static Stream createTurnByTurnTrace() { walkLeg, new TraceData( createPoint(pointAfterTurn, 0, calculateBearing(pointAfterTurn, virginiaAvenuePoint)), - NO_INSTRUCTION, + new ContinueInstruction(virginiaAvenueNortheastStep, locale).build(), false, - "After turn left on to Virginia Avenue should not produce turn instruction." + "After turn left on to Virginia Avenue should provide continue instruction." ) ), Arguments.of( From 6a1a94ab4b6f8ac32448c66db5b572cbeaf942c0 Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Wed, 20 Nov 2024 14:42:23 +0000 Subject: [PATCH 02/26] refactor(Reworked the logic to produce continue instruction): --- pom.xml | 7 -- .../triptracker/TravelerLocator.java | 36 ++++-- .../instruction/ContinueInstruction.java | 5 +- .../triptracker/ManageLegTraversalTest.java | 39 ++++++- .../baptist-church-to-east-crogan-street.json | 107 ++++++++++++++++++ 5 files changed, 173 insertions(+), 21 deletions(-) create mode 100644 src/test/resources/org/opentripplanner/middleware/controllers/api/baptist-church-to-east-crogan-street.json diff --git a/pom.xml b/pom.xml index 26fce5f3e..d0131965f 100644 --- a/pom.xml +++ b/pom.xml @@ -125,13 +125,6 @@ 2.7 - - - org.apache.commons - commons-lang3 - 3.17.0 - - org.slf4j diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TravelerLocator.java b/src/main/java/org/opentripplanner/middleware/triptracker/TravelerLocator.java index ce2fa62fe..5c7e5da32 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/TravelerLocator.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/TravelerLocator.java @@ -30,7 +30,6 @@ import java.util.stream.Collectors; import java.util.stream.IntStream; -import static org.apache.commons.lang3.ObjectUtils.isNotEmpty; import static org.opentripplanner.middleware.triptracker.instruction.TripInstruction.NO_INSTRUCTION; import static org.opentripplanner.middleware.triptracker.instruction.TripInstruction.TRIP_INSTRUCTION_IMMEDIATE_RADIUS; import static org.opentripplanner.middleware.triptracker.instruction.TripInstruction.TRIP_INSTRUCTION_UPCOMING_RADIUS; @@ -194,32 +193,49 @@ public static TripInstruction alignTravelerToTrip( locale ); } - return (travelerHasDeviated || (isNotEmpty(tripInstruction) && tripInstruction.hasInstruction())) + return (travelerHasDeviated || (tripInstruction != null && tripInstruction.hasInstruction())) ? tripInstruction : getContinueInstruction(travelerPosition, nextStep, locale); } /** * Traveler is on track, but no immediate instruction is available. Provide a "continue on street" reassurance - * instruction providing they are on a walk leg. This will be based on the current or previous step depending on the - * traveler's relative position to the next leg. + * instruction if the traveler is on a walk leg. This will be based on the next or previous step depending on the + * traveler's relative position to both. */ private static ContinueInstruction getContinueInstruction( TravelerPosition travelerPosition, Step nextStep, Locale locale ) { - if (!travelerPosition.expectedLeg.transitLeg && nextStep != null) { - Step currentStep = isPositionPastStep(travelerPosition, nextStep) - ? nextStep : - getPreviousStep(travelerPosition.expectedLeg.steps, nextStep); - if (currentStep != null) { - return new ContinueInstruction(currentStep, locale); + if ( + Boolean.TRUE.equals(!travelerPosition.expectedLeg.transitLeg) && + travelerPosition.expectedLeg.steps != null && + !travelerPosition.expectedLeg.steps.isEmpty() + ) { + Step previousStep = getPreviousStep(travelerPosition.expectedLeg.steps, nextStep); + if (previousStep != null) { + boolean travelerBetweenSteps = isPointBetween(previousStep.toCoordinates(), nextStep.toCoordinates(), travelerPosition.currentPosition); + if (travelerBetweenSteps) { + return new ContinueInstruction(previousStep, locale); + } else if (isWithinStepRange(travelerPosition, previousStep)) { + return new ContinueInstruction(previousStep, locale); + } else if (isWithinStepRange(travelerPosition, nextStep)) { + return new ContinueInstruction(nextStep, locale); + } } } return null; } + /** + * The traveler is still with the provided step range. + */ + private static boolean isWithinStepRange(TravelerPosition travelerPosition, Step step) { + double distanceFromTravelerToStep = getDistance(travelerPosition.currentPosition, step.toCoordinates()); + return distanceFromTravelerToStep < step.distance; + } + /** * Get the step prior to the next step provided. */ diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/instruction/ContinueInstruction.java b/src/main/java/org/opentripplanner/middleware/triptracker/instruction/ContinueInstruction.java index c0f6f6915..d961ec6d7 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/instruction/ContinueInstruction.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/instruction/ContinueInstruction.java @@ -1,11 +1,10 @@ package org.opentripplanner.middleware.triptracker.instruction; +import org.apache.logging.log4j.util.Strings; import org.opentripplanner.middleware.otp.response.Step; import java.util.Locale; -import static org.apache.commons.lang3.ObjectUtils.isNotEmpty; - public class ContinueInstruction extends SelfLegInstruction { public ContinueInstruction(Step legStep, Locale locale) { this.legStep = legStep; @@ -14,7 +13,7 @@ public ContinueInstruction(Step legStep, Locale locale) { @Override public String build() { - if (isNotEmpty(legStep) && isNotEmpty(legStep.streetName)) { + if (legStep != null && !Strings.isBlank(legStep.streetName)) { // TODO: i18n return String.format("Continue on %s", legStep.streetName); } diff --git a/src/test/java/org/opentripplanner/middleware/triptracker/ManageLegTraversalTest.java b/src/test/java/org/opentripplanner/middleware/triptracker/ManageLegTraversalTest.java index e3d919b4f..c7b546ed5 100644 --- a/src/test/java/org/opentripplanner/middleware/triptracker/ManageLegTraversalTest.java +++ b/src/test/java/org/opentripplanner/middleware/triptracker/ManageLegTraversalTest.java @@ -51,6 +51,7 @@ public class ManageLegTraversalTest { private static Itinerary midtownToAnsleyItinerary; private static List midtownToAnsleyIntermediateStops; private static Itinerary firstLegBusTransit; + private static Itinerary baptistChurchToEastCroganStreetIntinerary; private static final Locale locale = Locale.US; @@ -79,6 +80,10 @@ public static void setUp() throws IOException { CommonTestUtils.getTestResourceAsString("controllers/api/first-leg-transit.json"), Itinerary.class ); + baptistChurchToEastCroganStreetIntinerary = JsonUtils.getPOJOFromJSON( + CommonTestUtils.getTestResourceAsString("controllers/api/baptist-church-to-east-crogan-street.json"), + Itinerary.class + ); // Hold on to the original list of intermediate stops (some tests will overwrite it) midtownToAnsleyIntermediateStops = midtownToAnsleyItinerary.legs.get(1).intermediateStops; } @@ -209,6 +214,11 @@ private static Stream createTurnByTurnTrace() { Coordinates busStopCoords = new Coordinates(firstBusLeg.from); String busStopName = firstBusLeg.from.name; + Leg toEastCroganFirstLeg = baptistChurchToEastCroganStreetIntinerary.legs.get(0); + Step southClaytonSt = toEastCroganFirstLeg.steps.get(1); + Step eastCroganSt = toEastCroganFirstLeg.steps.get(2); + Coordinates pointOnSouthClaytonSt = new Coordinates(33.955561, -83.988204); + return Stream.of( Arguments.of( firstBusLeg, @@ -271,7 +281,7 @@ private static Stream createTurnByTurnTrace() { walkLeg, new TraceData( createPoint(virginiaCircleNortheastCoords, 12, SOUTH_WEST_BEARING), - new ContinueInstruction(ponceDeLeonPlaceNortheastStep, locale).build(), + new ContinueInstruction(virginiaCircleNortheastStep, locale).build(), false, "On track approaching second step, provide continue instruction." ) @@ -359,6 +369,33 @@ private static Stream createTurnByTurnTrace() { false, "On destination instruction." ) + ), + Arguments.of( + toEastCroganFirstLeg, + new TraceData( + pointOnSouthClaytonSt, + new ContinueInstruction(southClaytonSt, locale).build(), + false, + "On track passed second step and not near to next step, provide continue instruction for second step." + ) + ), + Arguments.of( + toEastCroganFirstLeg, + new TraceData( + createPoint(pointOnSouthClaytonSt, 12, NORTH_WEST_BEARING), + new ContinueInstruction(southClaytonSt, locale).build(), + false, + "On track a bit near to the next step, provide continue instruction for second step." + ) + ), + Arguments.of( + toEastCroganFirstLeg, + new TraceData( + createPoint(pointOnSouthClaytonSt, 72, NORTH_BEARING), + new ContinueInstruction(eastCroganSt, locale).build(), + false, + "On track passed next step, provide continue instruction for next step." + ) ) ); } diff --git a/src/test/resources/org/opentripplanner/middleware/controllers/api/baptist-church-to-east-crogan-street.json b/src/test/resources/org/opentripplanner/middleware/controllers/api/baptist-church-to-east-crogan-street.json new file mode 100644 index 000000000..0198cf5d0 --- /dev/null +++ b/src/test/resources/org/opentripplanner/middleware/controllers/api/baptist-church-to-east-crogan-street.json @@ -0,0 +1,107 @@ +{ + "duration": 478, + "startTime": 1731606660000, + "endTime": 1731607138000, + "walkTime": 478, + "transitTime": 0, + "waitingTime": 0, + "walkDistance": 0, + "walkLimitExceeded": false, + "elevationLost": 0, + "elevationGained": 0, + "transfers": 0, + "fare": null, + "legs": [ + { + "startTime": 1731606660000, + "endTime": 1731607138000, + "departureDelay": 0, + "arrivalDelay": 0, + "realTime": false, + "distance": 636.31, + "mode": "WALK", + "interlineWithPreviousLeg": false, + "from": { + "name": "First Baptist Church of Lawrenceville, Lawrenceville, GA, USA", + "lon": -83.9881534, + "lat": 33.9549491, + "vertexType": "NORMAL" + }, + "to": { + "name": "49 East Crogan Street, Lawrenceville, GA, USA", + "lon": -83.9833418, + "lat": 33.9565218, + "vertexType": "NORMAL" + }, + "legGeometry": { + "points": "myfnEz|r_OG@G?A?wBLI?I@_@?{@DG?Ek@GeBCk@G?I?I?A?AK?G?E?a@?]?AAE?G?M?M?E?EGwB?EAEAGAC?A@?@Y@[?C@@H@L@@?ACAAAKAQ?U?KAKAeA?O?K?KA]?K?KAg@?IAO?KE{B", + "length": 62 + }, + "rentedBike": false, + "transitLeg": false, + "duration": 478, + "steps": [ + { + "distance": 10.11, + "relativeDirection": "DEPART", + "streetName": "crossing over Luckie Street☆☆☆", + "absoluteDirection": "NORTH", + "stayOn": false, + "area": false, + "lon": -83.9881362, + "lat": 33.9549515 + }, + { + "distance": 134.71, + "relativeDirection": "CONTINUE", + "streetName": "South Clayton Street★★★", + "absoluteDirection": "NORTH", + "stayOn": false, + "area": false, + "lon": -83.9881458, + "lat": 33.955042 + }, + { + "distance": 88.77, + "relativeDirection": "RIGHT", + "streetName": "East Crogan Street★★★", + "absoluteDirection": "EAST", + "stayOn": true, + "area": false, + "lon": -83.9882578, + "lat": 33.9562495 + }, + { + "distance": 17.43, + "relativeDirection": "LEFT", + "streetName": "crossing over East Crogan Street☆☆☆", + "absoluteDirection": "NORTH", + "stayOn": true, + "area": false, + "lon": -83.9873003, + "lat": 33.9563302 + }, + { + "distance": 177.64, + "relativeDirection": "RIGHT", + "streetName": "East Crogan Street★★★", + "absoluteDirection": "EAST", + "stayOn": false, + "area": false, + "lon": -83.9873097, + "lat": 33.9564868 + }, + { + "distance": 207.65, + "relativeDirection": "LEFT", + "streetName": "East Crogan Street★★★", + "absoluteDirection": "EAST", + "stayOn": true, + "area": false, + "lon": -83.985582, + "lat": 33.9564082 + } + ] + } + ] +} \ No newline at end of file From bd9f99c3189cc789bf3665e4f9376b6839cad7ab Mon Sep 17 00:00:00 2001 From: Robin Beer Date: Wed, 20 Nov 2024 14:50:28 +0000 Subject: [PATCH 03/26] refactor(Fixed failing test): --- .../middleware/controllers/api/TrackedTripControllerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 25e05d315..9124556f7 100644 --- a/src/test/java/org/opentripplanner/middleware/controllers/api/TrackedTripControllerTest.java +++ b/src/test/java/org/opentripplanner/middleware/controllers/api/TrackedTripControllerTest.java @@ -351,7 +351,7 @@ private static Stream createInstructionAndStatusCases() { Arguments.of( monitoredTrip, createPoint(firstStepCoords, 20, WEST_BEARING), - new ContinueInstruction(virginiaCircleNortheastStep, locale).build(), + new ContinueInstruction(adairAvenueNortheastStep, locale).build(), TripStatus.ON_SCHEDULE, "Coords along a step should produce a continue on street instruction" ), From 1c1c972fb8fc3aa996a1c86375426e13d9b1cc8f Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 20 Nov 2024 19:08:34 -0500 Subject: [PATCH 04/26] chore: Add email templates for trip sharing. --- src/main/resources/templates/ShareTripHtml.ftl | 15 +++++++++++++++ src/main/resources/templates/ShareTripText.ftl | 11 +++++++++++ 2 files changed, 26 insertions(+) create mode 100644 src/main/resources/templates/ShareTripHtml.ftl create mode 100644 src/main/resources/templates/ShareTripText.ftl diff --git a/src/main/resources/templates/ShareTripHtml.ftl b/src/main/resources/templates/ShareTripHtml.ftl new file mode 100644 index 000000000..999ceef8b --- /dev/null +++ b/src/main/resources/templates/ShareTripHtml.ftl @@ -0,0 +1,15 @@ +<#ftl auto_esc=false> +<#include "OtpUserContainer.ftl"> + +<#-- + This is a template for an HTML email that gets sent when a dependent user is requesting a trusted companion. +--> + +<#macro EmailMain> +
+

${emailGreeting}

+

${tripLinkAnchorLabel}

+
+ + +<@HtmlEmail/> \ No newline at end of file diff --git a/src/main/resources/templates/ShareTripText.ftl b/src/main/resources/templates/ShareTripText.ftl new file mode 100644 index 000000000..c87ed5a0c --- /dev/null +++ b/src/main/resources/templates/ShareTripText.ftl @@ -0,0 +1,11 @@ +<#-- + This is a template for a text email that gets sent when a dependent requests a trusted companion. + + Note: in plain text emails, all whitespace is preserved, + so the indentation of the notification content is intentionally not aligned + with the indentation of the macros. + +--> +${emailGreeting} + +${tripLinkLabelAndUrl} \ No newline at end of file From 9c3eba8c9717634f5f93164ee3c287a2185f31d3 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 22 Nov 2024 16:59:10 -0500 Subject: [PATCH 05/26] Resolve conflicts --- .../api/MonitoredTripController.java | 88 +++++++++++++++++++ .../middleware/models/MonitoredTrip.java | 13 +++ .../tripmonitor/jobs/CheckMonitoredTrip.java | 8 +- 3 files changed, 102 insertions(+), 7 deletions(-) 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 49b854425..d643c4096 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java @@ -5,26 +5,37 @@ import com.mongodb.client.model.Filters; import org.bson.conversions.Bson; import org.eclipse.jetty.http.HttpStatus; +import org.opentripplanner.middleware.i18n.Message; import org.opentripplanner.middleware.models.ItineraryExistence; import org.opentripplanner.middleware.models.MonitoredTrip; import org.opentripplanner.middleware.models.OtpUser; import org.opentripplanner.middleware.persistence.Persistence; +import org.opentripplanner.middleware.models.RelatedUser; import org.opentripplanner.middleware.tripmonitor.jobs.CheckMonitoredTrip; import org.opentripplanner.middleware.tripmonitor.jobs.MonitoredTripLocks; +import org.opentripplanner.middleware.utils.ConfigUtils; import org.opentripplanner.middleware.utils.InvalidItineraryReason; import org.opentripplanner.middleware.utils.JsonUtils; +import org.opentripplanner.middleware.utils.NotificationUtils; import org.opentripplanner.middleware.utils.SwaggerUtils; import spark.Request; import spark.Response; +import java.util.ArrayList; +import java.util.List; +import java.util.Locale; +import java.util.Map; import java.util.Set; import java.util.stream.Collectors; 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.tripmonitor.jobs.CheckMonitoredTrip.SETTINGS_PATH; import static org.opentripplanner.middleware.utils.ConfigUtils.getConfigPropertyAsInt; import static org.opentripplanner.middleware.utils.HttpUtils.JSON_ONLY; +import static org.opentripplanner.middleware.utils.I18nUtils.getOtpUserLocale; +import static org.opentripplanner.middleware.utils.I18nUtils.label; import static org.opentripplanner.middleware.utils.JsonUtils.getPOJOFromRequestBody; import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt; @@ -36,6 +47,10 @@ public class MonitoredTripController extends ApiController { private static final int MAXIMUM_PERMITTED_MONITORED_TRIPS = getConfigPropertyAsInt("MAXIMUM_PERMITTED_MONITORED_TRIPS", 5); + private final String OTP_UI_NAME = ConfigUtils.getConfigPropertyAsText("OTP_UI_NAME"); + + private static final String OTP_UI_URL = ConfigUtils.getConfigPropertyAsText("OTP_UI_URL"); + public MonitoredTripController(String apiPrefix) { super(apiPrefix, Persistence.monitoredTrips, "secure/monitoredtrip"); } @@ -90,6 +105,8 @@ MonitoredTrip preCreateHook(MonitoredTrip monitoredTrip, Request req) { } } + notifyTripCompanionsAndObservers(monitoredTrip, null); + return monitoredTrip; } @@ -128,6 +145,74 @@ private void preCreateOrUpdateChecks(MonitoredTrip monitoredTrip, Request req) { processTripQueryParams(monitoredTrip, req); } + /** Notify users added as companions or observers to a trip. (Removed users won't get notified.) */ + private void notifyTripCompanionsAndObservers(MonitoredTrip monitoredTrip, MonitoredTrip originalTrip) { + RelatedUser addedCompanion = null; + List addedObservers = new ArrayList<>(); + if (monitoredTrip.companion != null) { + if (originalTrip == null || originalTrip.companion == null) { + // notify everyone + addedCompanion = monitoredTrip.companion; + } else { + // notify added companions + if (!originalTrip.companion.email.equals(monitoredTrip.companion.email)) { + addedCompanion = monitoredTrip.companion; + } + } + } + + if (monitoredTrip.observers != null) { + if (originalTrip == null || originalTrip.observers == null) { + // notify everyone + addedObservers.addAll(monitoredTrip.observers); + } else { + // notify added observers + Set existingObserverEmails = originalTrip.observers.stream() + .map(obs -> obs.email) + .collect(Collectors.toSet()); + monitoredTrip.observers.forEach(obs -> { + if (!existingObserverEmails.contains(obs.email)) { + addedObservers.add(obs); + } + }); + } + } + + if (addedCompanion != null) { + OtpUser companionUser = Persistence.otpUsers.getOneFiltered(Filters.eq("email", addedCompanion.email)); + if (companionUser != null) { + Locale locale = getOtpUserLocale(companionUser); + String tripLinkLabel = Message.TRIP_LINK_TEXT.get(locale); + String tripUrl = monitoredTrip.getTripUrl(); + + OtpUser tripCreator = Persistence.otpUsers.getById(monitoredTrip.userId); + + // TODO: finish i18n + // TODO: Refactor common email data. + NotificationUtils.sendEmail( + companionUser, + String.format("%s: %s shared a trip with you.", OTP_UI_NAME, tripCreator.email), + "ShareTripText.ftl", + "ShareTripHtml.ftl", + Map.of( + "emailGreeting", String.format( + "You are a companion for %s on a trip from %s to %s.", + tripCreator.email, + monitoredTrip.from.name, + monitoredTrip.to.name + ), + "tripUrl", tripUrl, + "tripLinkAnchorLabel", tripLinkLabel, + "tripLinkLabelAndUrl", label(tripLinkLabel, tripUrl, locale), + "emailFooter", String.format(Message.TRIP_EMAIL_FOOTER.get(locale), OTP_UI_NAME), + "manageLinkText", Message.TRIP_EMAIL_MANAGE_NOTIFICATIONS.get(locale), + "manageLinkUrl", String.format("%s%s", OTP_UI_URL, SETTINGS_PATH) + ) + ); + } + } + } + /** * Processes the {@link MonitoredTrip} query parameters, so the trip's fields match the query parameters. * If an error occurs regarding the query params, returns a HTTP 400 status. @@ -171,6 +256,9 @@ MonitoredTrip preUpdateHook(MonitoredTrip monitoredTrip, MonitoredTrip preExisti // perform the database update here before releasing the lock to be sure that the record is updated in the // database before a CheckMonitoredTripJob analyzes the data Persistence.monitoredTrips.replace(monitoredTrip.id, monitoredTrip); + + notifyTripCompanionsAndObservers(monitoredTrip, preExisting); + return runCheckMonitoredTrip(monitoredTrip); } catch (Exception e) { // FIXME: an error happened while updating the trip, but the trip might have been saved to the DB, so return diff --git a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java index 1626ca8fd..da946c7f0 100644 --- a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java @@ -17,6 +17,7 @@ import org.opentripplanner.middleware.persistence.Persistence; import org.opentripplanner.middleware.persistence.TypedPersistence; import org.opentripplanner.middleware.tripmonitor.JourneyState; +import org.opentripplanner.middleware.utils.ConfigUtils; import org.opentripplanner.middleware.utils.DateTimeUtils; import org.opentripplanner.middleware.utils.ItineraryUtils; import spark.Request; @@ -29,6 +30,8 @@ import java.util.List; import java.util.function.Function; +import static org.opentripplanner.middleware.tripmonitor.jobs.CheckMonitoredTrip.ACCOUNT_PATH; + /** * A monitored trip represents a trip a user would like to receive notification on if affected by a delay and/or route * change. @@ -38,6 +41,10 @@ public class MonitoredTrip extends Model { public static final String USER_ID_FIELD_NAME = "userId"; + private final String TRIPS_PATH = ACCOUNT_PATH + "/trips"; + + private static final String OTP_UI_URL = ConfigUtils.getConfigPropertyAsText("OTP_UI_URL"); + /** * Mongo Id of the {@link OtpUser} who owns this monitored trip. */ @@ -427,4 +434,10 @@ public int tripTimeMinute() { public boolean isOneTime() { return !monday && !tuesday && !wednesday && !thursday && !friday && !saturday && !sunday; } + + @JsonIgnore + @BsonIgnore + public String getTripUrl() { + return String.format("%s%s/%s", OTP_UI_URL, TRIPS_PATH, id); + } } diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java index d8c44769f..a2be9347e 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java @@ -59,8 +59,6 @@ public class CheckMonitoredTrip implements Runnable { public static final String ACCOUNT_PATH = "/#/account"; - private final String TRIPS_PATH = ACCOUNT_PATH + "/trips"; - public static final String SETTINGS_PATH = ACCOUNT_PATH + "/settings"; public final MonitoredTrip trip; @@ -530,7 +528,7 @@ private void sendNotifications() { Locale locale = getOtpUserLocale(); String tripLinkLabel = Message.TRIP_LINK_TEXT.get(locale); - String tripUrl = getTripUrl(); + String tripUrl = trip.getTripUrl(); // A HashMap is needed instead of a Map for template data to be serialized to the template renderer. Map templateData = new HashMap<>(Map.of( "emailGreeting", Message.TRIP_EMAIL_GREETING.get(locale), @@ -925,8 +923,4 @@ private OtpUser getOtpUser() { private Locale getOtpUserLocale() { return I18nUtils.getOtpUserLocale(getOtpUser()); } - - private String getTripUrl() { - return String.format("%s%s/%s", OTP_UI_URL, TRIPS_PATH, trip.id); - } } From eb5c8e255057f436108a4632ce928c52cdf4de15 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Mon, 25 Nov 2024 13:22:38 -0500 Subject: [PATCH 06/26] refactor(MonitoredTrip): Add logic for extracting added users. --- .../api/MonitoredTripController.java | 123 +++++++++--------- .../middleware/models/MonitoredTrip.java | 69 ++++++++++ .../middleware/models/MonitoredTripTest.java | 98 ++++++++++++++ 3 files changed, 231 insertions(+), 59 deletions(-) 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 d643c4096..ad1559a2a 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java @@ -21,8 +21,6 @@ import spark.Request; import spark.Response; -import java.util.ArrayList; -import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; @@ -31,6 +29,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.models.MonitoredTrip.getAddedUsers; import static org.opentripplanner.middleware.tripmonitor.jobs.CheckMonitoredTrip.SETTINGS_PATH; import static org.opentripplanner.middleware.utils.ConfigUtils.getConfigPropertyAsInt; import static org.opentripplanner.middleware.utils.HttpUtils.JSON_ONLY; @@ -51,6 +50,12 @@ public class MonitoredTripController extends ApiController { private static final String OTP_UI_URL = ConfigUtils.getConfigPropertyAsText("OTP_UI_URL"); + private enum UserType { + COMPANION, + OBSERVER, + PRIMARY_TRAVELER + } + public MonitoredTripController(String apiPrefix) { super(apiPrefix, Persistence.monitoredTrips, "secure/monitoredtrip"); } @@ -147,69 +152,69 @@ private void preCreateOrUpdateChecks(MonitoredTrip monitoredTrip, Request req) { /** Notify users added as companions or observers to a trip. (Removed users won't get notified.) */ private void notifyTripCompanionsAndObservers(MonitoredTrip monitoredTrip, MonitoredTrip originalTrip) { - RelatedUser addedCompanion = null; - List addedObservers = new ArrayList<>(); - if (monitoredTrip.companion != null) { - if (originalTrip == null || originalTrip.companion == null) { - // notify everyone - addedCompanion = monitoredTrip.companion; - } else { - // notify added companions - if (!originalTrip.companion.email.equals(monitoredTrip.companion.email)) { - addedCompanion = monitoredTrip.companion; - } - } + MonitoredTrip.TripUsers usersToNotify = getAddedUsers(monitoredTrip, originalTrip); + + if (usersToNotify.companion != null) { + OtpUser companionUser = Persistence.otpUsers.getOneFiltered(Filters.eq("email", usersToNotify.companion.email)); + notifyCompanion(monitoredTrip, companionUser, UserType.COMPANION); + } + + if (usersToNotify.primary != null) { + // email could be used too for primary users + OtpUser primaryUser = Persistence.otpUsers.getById(usersToNotify.primary.userId); + notifyCompanion(monitoredTrip, primaryUser, UserType.PRIMARY_TRAVELER); } - if (monitoredTrip.observers != null) { - if (originalTrip == null || originalTrip.observers == null) { - // notify everyone - addedObservers.addAll(monitoredTrip.observers); - } else { - // notify added observers - Set existingObserverEmails = originalTrip.observers.stream() - .map(obs -> obs.email) - .collect(Collectors.toSet()); - monitoredTrip.observers.forEach(obs -> { - if (!existingObserverEmails.contains(obs.email)) { - addedObservers.add(obs); - } - }); + if (!usersToNotify.observers.isEmpty()) { + for (RelatedUser observer : usersToNotify.observers) { + OtpUser observerUser = Persistence.otpUsers.getOneFiltered(Filters.eq("email", observer.email)); + notifyCompanion(monitoredTrip, observerUser, UserType.OBSERVER); } } + } - if (addedCompanion != null) { - OtpUser companionUser = Persistence.otpUsers.getOneFiltered(Filters.eq("email", addedCompanion.email)); - if (companionUser != null) { - Locale locale = getOtpUserLocale(companionUser); - String tripLinkLabel = Message.TRIP_LINK_TEXT.get(locale); - String tripUrl = monitoredTrip.getTripUrl(); - - OtpUser tripCreator = Persistence.otpUsers.getById(monitoredTrip.userId); - - // TODO: finish i18n - // TODO: Refactor common email data. - NotificationUtils.sendEmail( - companionUser, - String.format("%s: %s shared a trip with you.", OTP_UI_NAME, tripCreator.email), - "ShareTripText.ftl", - "ShareTripHtml.ftl", - Map.of( - "emailGreeting", String.format( - "You are a companion for %s on a trip from %s to %s.", - tripCreator.email, - monitoredTrip.from.name, - monitoredTrip.to.name - ), - "tripUrl", tripUrl, - "tripLinkAnchorLabel", tripLinkLabel, - "tripLinkLabelAndUrl", label(tripLinkLabel, tripUrl, locale), - "emailFooter", String.format(Message.TRIP_EMAIL_FOOTER.get(locale), OTP_UI_NAME), - "manageLinkText", Message.TRIP_EMAIL_MANAGE_NOTIFICATIONS.get(locale), - "manageLinkUrl", String.format("%s%s", OTP_UI_URL, SETTINGS_PATH) - ) - ); + private void notifyCompanion(MonitoredTrip monitoredTrip, OtpUser companionUser, UserType userType) { + if (companionUser != null) { + Locale locale = getOtpUserLocale(companionUser); + String tripLinkLabel = Message.TRIP_LINK_TEXT.get(locale); + String tripUrl = monitoredTrip.getTripUrl(); + + OtpUser tripCreator = Persistence.otpUsers.getById(monitoredTrip.userId); + + String greeting; + switch (userType) { + case COMPANION: + greeting = "%s added you as a companion on their trip:"; + break; + case PRIMARY_TRAVELER: + greeting = "%s made you the primary traveler on this trip:"; + break; + case OBSERVER: + default: + greeting = "%s added you as an observer for their trip:"; + break; } + + // TODO: finish i18n + NotificationUtils.sendEmail( + companionUser, + // TODO: Set the sender as the user who added you to their trip (like GitHub, Jira, etc) + monitoredTrip.tripName + " Notification", // TODO: Reuse trip monitor notification subject + "ShareTripText.ftl", // TODO: See if msg body can be reused + "ShareTripHtml.ftl", + Map.of( + "emailGreeting", String.format( + greeting, + tripCreator.email + ), + "tripUrl", tripUrl, + "tripLinkAnchorLabel", tripLinkLabel, + "tripLinkLabelAndUrl", label(tripLinkLabel, tripUrl, locale), + "emailFooter", String.format(Message.TRIP_EMAIL_FOOTER.get(locale), OTP_UI_NAME), + "manageLinkText", Message.TRIP_EMAIL_MANAGE_NOTIFICATIONS.get(locale), + "manageLinkUrl", String.format("%s%s", OTP_UI_URL, SETTINGS_PATH) + ) + ); } } diff --git a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java index da946c7f0..6acb69c54 100644 --- a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java @@ -28,7 +28,9 @@ import java.time.ZonedDateTime; import java.util.ArrayList; import java.util.List; +import java.util.Set; import java.util.function.Function; +import java.util.stream.Collectors; import static org.opentripplanner.middleware.tripmonitor.jobs.CheckMonitoredTrip.ACCOUNT_PATH; @@ -440,4 +442,71 @@ public boolean isOneTime() { public String getTripUrl() { return String.format("%s%s/%s", OTP_UI_URL, TRIPS_PATH, id); } + + /** + * Gets users not previously involved (as primary traveler, companion, or observer) in a trip. + */ + public static TripUsers getAddedUsers(MonitoredTrip monitoredTrip, MonitoredTrip originalTrip) { + RelatedUser addedCompanion = null; + if (monitoredTrip.companion != null && monitoredTrip.companion.status == RelatedUser.RelatedUserStatus.CONFIRMED) { + if (originalTrip == null || originalTrip.companion == null) { + // notify added companion if creating trip or setting companion for the first time + addedCompanion = monitoredTrip.companion; + } else { + // notify added companion but not the previous one. + if (!originalTrip.companion.email.equals(monitoredTrip.companion.email)) { + addedCompanion = monitoredTrip.companion; + } + } + } + + MobilityProfileLite addedPrimaryTraveler = null; + if (monitoredTrip.primary != null) { + if (originalTrip == null || originalTrip.primary == null) { + // notify everyone + addedPrimaryTraveler = monitoredTrip.primary; + } else { + // notify added traveler + // email could be used too be primary travelers + if (!originalTrip.primary.userId.equals(monitoredTrip.primary.userId)) { + addedPrimaryTraveler = monitoredTrip.primary; + } + } + } + + List addedObservers = new ArrayList<>(); + if (monitoredTrip.observers != null) { + List confirmedObservers = monitoredTrip.observers.stream() + .filter(o -> o.status == RelatedUser.RelatedUserStatus.CONFIRMED) + .collect(Collectors.toList()); + if (originalTrip == null || originalTrip.observers == null) { + // notify everyone + addedObservers.addAll(confirmedObservers); + } else { + // notify added observers + Set existingObserverEmails = originalTrip.observers.stream() + .map(obs -> obs.email) + .collect(Collectors.toSet()); + confirmedObservers.forEach(obs -> { + if (!existingObserverEmails.contains(obs.email)) { + addedObservers.add(obs); + } + }); + } + } + + return new TripUsers(addedPrimaryTraveler, addedCompanion, addedObservers); + } + + public static class TripUsers { + public final RelatedUser companion; + public final List observers; + public final MobilityProfileLite primary; + + public TripUsers(MobilityProfileLite primary, RelatedUser companion, List observers) { + this.primary = primary; + this.companion = companion; + this.observers = observers; + } + } } diff --git a/src/test/java/org/opentripplanner/middleware/models/MonitoredTripTest.java b/src/test/java/org/opentripplanner/middleware/models/MonitoredTripTest.java index 4ba076646..c24ad07d1 100644 --- a/src/test/java/org/opentripplanner/middleware/models/MonitoredTripTest.java +++ b/src/test/java/org/opentripplanner/middleware/models/MonitoredTripTest.java @@ -1,12 +1,16 @@ package org.opentripplanner.middleware.models; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.opentripplanner.middleware.otp.OtpGraphQLTransportMode; import org.opentripplanner.middleware.otp.OtpGraphQLVariables; import org.opentripplanner.middleware.otp.response.Itinerary; import org.opentripplanner.middleware.otp.response.Leg; import org.opentripplanner.middleware.otp.response.Place; +import java.util.ArrayList; import java.util.List; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -41,4 +45,98 @@ void initializeFromItineraryAndQueryParamsShouldNotModifyModes() { trip.initializeFromItineraryAndQueryParams(variables); assertEquals(originalModes, trip.otp2QueryParams.modes); } + + @ParameterizedTest + @MethodSource("createGetAddedUsersCases") + void canGetAddedUsers(MonitoredTrip.TripUsers originalUsers, MonitoredTrip.TripUsers finalUsers, MonitoredTrip.TripUsers expected) { + MonitoredTrip originalTrip = new MonitoredTrip(); + originalTrip.primary = originalUsers.primary; + originalTrip.companion = originalUsers.companion; + originalTrip.observers = originalUsers.observers; + + MonitoredTrip finalTrip = new MonitoredTrip(); + finalTrip.primary = finalUsers.primary; + finalTrip.companion = finalUsers.companion; + finalTrip.observers = finalUsers.observers; + + MonitoredTrip.TripUsers tripUsers = MonitoredTrip.getAddedUsers(finalTrip, originalTrip); + + assertEquals(expected.primary != null, tripUsers.primary != null); + if (expected.primary != null && tripUsers.primary != null) { + assertEquals(expected.primary.userId, tripUsers.primary.userId); + } + assertEquals(expected.companion != null, tripUsers.companion != null); + if (expected.companion != null && tripUsers.companion != null) { + assertEquals(expected.companion.email, tripUsers.companion.email); + } + assertEquals(expected.observers, tripUsers.observers); + } + + private static Stream createGetAddedUsersCases() { + MobilityProfileLite primary = new MobilityProfileLite(); + primary.userId = "primary-user-id"; + + MobilityProfileLite newPrimary = new MobilityProfileLite(); + primary.userId = "new-primary-user-id"; + + RelatedUser companion = new RelatedUser(); + companion.email = "companion@example.com"; + companion.status = RelatedUser.RelatedUserStatus.CONFIRMED; + + RelatedUser newCompanion = new RelatedUser(); + newCompanion.email = "new-companion@example.com"; + newCompanion.status = RelatedUser.RelatedUserStatus.CONFIRMED; + + + RelatedUser unconfirmedCompanion = new RelatedUser(); + unconfirmedCompanion.email = "unconfirmed-companion@example.com"; + unconfirmedCompanion.status = RelatedUser.RelatedUserStatus.PENDING; + + RelatedUser observer1 = new RelatedUser(); + observer1.email = "observer1@example.com"; + observer1.status = RelatedUser.RelatedUserStatus.CONFIRMED; + + RelatedUser observer2 = new RelatedUser(); + observer2.email = "observer2@example.com"; + observer2.status = RelatedUser.RelatedUserStatus.CONFIRMED; + + RelatedUser observer3 = new RelatedUser(); + observer3.email = "observer3@example.com"; + observer3.status = RelatedUser.RelatedUserStatus.CONFIRMED; + + List observers = List.of(observer1, observer2); + + return Stream.of( + // If the final trip has the same users as the original one, no one has been added. + Arguments.of( + new MonitoredTrip.TripUsers(primary, companion, observers), + new MonitoredTrip.TripUsers(primary, companion, observers), + new MonitoredTrip.TripUsers(null, null, new ArrayList<>()) + ), + // If the final trip drops original users without adding new ones, no one has been added. + Arguments.of( + new MonitoredTrip.TripUsers(primary, companion, observers), + new MonitoredTrip.TripUsers(null, null, List.of()), + new MonitoredTrip.TripUsers(null, null, new ArrayList<>()) + ), + // If the original trip did not include users, users in the final trip have been added. + Arguments.of( + new MonitoredTrip.TripUsers(null, null, List.of()), + new MonitoredTrip.TripUsers(primary, companion, observers), + new MonitoredTrip.TripUsers(primary, companion, observers) + ), + // If users have been modified, the modified users should appear in added users. + Arguments.of( + new MonitoredTrip.TripUsers(primary, companion, observers), + new MonitoredTrip.TripUsers(newPrimary, newCompanion, List.of(observer1, observer3)), + new MonitoredTrip.TripUsers(newPrimary, newCompanion, List.of(observer3)) + ), + // If users have been modified, unconfirmed users should not be added. + Arguments.of( + new MonitoredTrip.TripUsers(primary, companion, observers), + new MonitoredTrip.TripUsers(newPrimary, unconfirmedCompanion, List.of(observer1, unconfirmedCompanion)), + new MonitoredTrip.TripUsers(newPrimary, null, List.of()) + ) + ); + } } From 1e6c4aa48cc6eb2385ed9358fefd560b81eb9f6e Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Mon, 25 Nov 2024 13:31:14 -0500 Subject: [PATCH 07/26] test(MonitoredTrip): Refactor tests. --- .../middleware/models/RelatedUser.java | 4 +++ .../middleware/models/MonitoredTripTest.java | 31 +++++-------------- 2 files changed, 12 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java b/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java index 920e94eda..ea8e2f8e8 100644 --- a/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java @@ -15,6 +15,10 @@ public RelatedUser() { // Required for JSON deserialization. } + public RelatedUser(String email, RelatedUserStatus status) { + this(email, status, null); + } + public RelatedUser(String email, RelatedUserStatus status, String nickname) { this.email = email; this.status = status; diff --git a/src/test/java/org/opentripplanner/middleware/models/MonitoredTripTest.java b/src/test/java/org/opentripplanner/middleware/models/MonitoredTripTest.java index c24ad07d1..5063a6980 100644 --- a/src/test/java/org/opentripplanner/middleware/models/MonitoredTripTest.java +++ b/src/test/java/org/opentripplanner/middleware/models/MonitoredTripTest.java @@ -16,6 +16,8 @@ import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.opentripplanner.middleware.models.RelatedUser.RelatedUserStatus.CONFIRMED; +import static org.opentripplanner.middleware.models.RelatedUser.RelatedUserStatus.PENDING; class MonitoredTripTest { @Test @@ -79,30 +81,13 @@ private static Stream createGetAddedUsersCases() { MobilityProfileLite newPrimary = new MobilityProfileLite(); primary.userId = "new-primary-user-id"; - RelatedUser companion = new RelatedUser(); - companion.email = "companion@example.com"; - companion.status = RelatedUser.RelatedUserStatus.CONFIRMED; + RelatedUser companion = new RelatedUser("companion@example.com", CONFIRMED); + RelatedUser newCompanion = new RelatedUser("new-companion@example.com", CONFIRMED); + RelatedUser unconfirmedCompanion = new RelatedUser("unconfirmed-companion@example.com", PENDING); - RelatedUser newCompanion = new RelatedUser(); - newCompanion.email = "new-companion@example.com"; - newCompanion.status = RelatedUser.RelatedUserStatus.CONFIRMED; - - - RelatedUser unconfirmedCompanion = new RelatedUser(); - unconfirmedCompanion.email = "unconfirmed-companion@example.com"; - unconfirmedCompanion.status = RelatedUser.RelatedUserStatus.PENDING; - - RelatedUser observer1 = new RelatedUser(); - observer1.email = "observer1@example.com"; - observer1.status = RelatedUser.RelatedUserStatus.CONFIRMED; - - RelatedUser observer2 = new RelatedUser(); - observer2.email = "observer2@example.com"; - observer2.status = RelatedUser.RelatedUserStatus.CONFIRMED; - - RelatedUser observer3 = new RelatedUser(); - observer3.email = "observer3@example.com"; - observer3.status = RelatedUser.RelatedUserStatus.CONFIRMED; + RelatedUser observer1 = new RelatedUser("observer1@example.com", CONFIRMED); + RelatedUser observer2 = new RelatedUser("observer2@example.com", CONFIRMED); + RelatedUser observer3 = new RelatedUser("observer3@example.com", CONFIRMED); List observers = List.of(observer1, observer2); From 4cc7b6f7586b377f03baf58888d0ca540982de6c Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Mon, 25 Nov 2024 14:15:25 -0500 Subject: [PATCH 08/26] refactor(RelatedUser): Add isConfirmed method --- .../org/opentripplanner/middleware/models/MonitoredTrip.java | 4 ++-- .../org/opentripplanner/middleware/models/RelatedUser.java | 4 ++++ .../middleware/controllers/api/OtpUserControllerTest.java | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java index 6acb69c54..69a99be99 100644 --- a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java @@ -448,7 +448,7 @@ public String getTripUrl() { */ public static TripUsers getAddedUsers(MonitoredTrip monitoredTrip, MonitoredTrip originalTrip) { RelatedUser addedCompanion = null; - if (monitoredTrip.companion != null && monitoredTrip.companion.status == RelatedUser.RelatedUserStatus.CONFIRMED) { + if (monitoredTrip.companion != null && monitoredTrip.companion.isConfirmed()) { if (originalTrip == null || originalTrip.companion == null) { // notify added companion if creating trip or setting companion for the first time addedCompanion = monitoredTrip.companion; @@ -477,7 +477,7 @@ public static TripUsers getAddedUsers(MonitoredTrip monitoredTrip, MonitoredTrip List addedObservers = new ArrayList<>(); if (monitoredTrip.observers != null) { List confirmedObservers = monitoredTrip.observers.stream() - .filter(o -> o.status == RelatedUser.RelatedUserStatus.CONFIRMED) + .filter(RelatedUser::isConfirmed) .collect(Collectors.toList()); if (originalTrip == null || originalTrip.observers == null) { // notify everyone diff --git a/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java b/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java index ea8e2f8e8..47ad86f83 100644 --- a/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java @@ -29,5 +29,9 @@ public RelatedUser(String email, RelatedUserStatus status, String nickname, Stri this (email, status, nickname); this.acceptKey = acceptKey; } + + public boolean isConfirmed() { + return status == RelatedUserStatus.CONFIRMED; + } } diff --git a/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java b/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java index a1d9f0c9e..758885c28 100644 --- a/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java +++ b/src/test/java/org/opentripplanner/middleware/controllers/api/OtpUserControllerTest.java @@ -212,7 +212,7 @@ void canAcceptDependentRequest() { relatedUsers .stream() .filter(user -> user.email.equals(relatedUserOne.email)) - .forEach(user -> assertEquals(RelatedUser.RelatedUserStatus.CONFIRMED, user.status)); + .forEach(user -> assertTrue(user.isConfirmed())); } @Test From b0015b278ef04469b6b20255694536d75fd13e91 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Mon, 25 Nov 2024 18:21:02 -0500 Subject: [PATCH 09/26] refactor(MonitoredTrip): Reduce getAddedUsers complexity level --- .../middleware/models/MonitoredTrip.java | 37 ++++++++----------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java index 69a99be99..39f4e31ba 100644 --- a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java @@ -448,30 +448,25 @@ public String getTripUrl() { */ public static TripUsers getAddedUsers(MonitoredTrip monitoredTrip, MonitoredTrip originalTrip) { RelatedUser addedCompanion = null; - if (monitoredTrip.companion != null && monitoredTrip.companion.isConfirmed()) { - if (originalTrip == null || originalTrip.companion == null) { - // notify added companion if creating trip or setting companion for the first time - addedCompanion = monitoredTrip.companion; - } else { - // notify added companion but not the previous one. - if (!originalTrip.companion.email.equals(monitoredTrip.companion.email)) { - addedCompanion = monitoredTrip.companion; - } - } + if (monitoredTrip.companion != null && monitoredTrip.companion.isConfirmed() && ( + originalTrip == null || + originalTrip.companion == null || + !originalTrip.companion.email.equals(monitoredTrip.companion.email) + )) { + // notify added companion if creating trip or setting companion for the first time, + // or setting a different companion. + addedCompanion = monitoredTrip.companion; } MobilityProfileLite addedPrimaryTraveler = null; - if (monitoredTrip.primary != null) { - if (originalTrip == null || originalTrip.primary == null) { - // notify everyone - addedPrimaryTraveler = monitoredTrip.primary; - } else { - // notify added traveler - // email could be used too be primary travelers - if (!originalTrip.primary.userId.equals(monitoredTrip.primary.userId)) { - addedPrimaryTraveler = monitoredTrip.primary; - } - } + if (monitoredTrip.primary != null && ( + originalTrip == null || + originalTrip.primary == null || + !originalTrip.primary.userId.equals(monitoredTrip.primary.userId) + )) { + // notify added primary traveler if creating trip or setting primary traveler for the first time, + // or setting a different primary traveler. + addedPrimaryTraveler = monitoredTrip.primary; } List addedObservers = new ArrayList<>(); From 147d2ab15aed4daf2f10ce86d9093ed630ed994b Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Mon, 25 Nov 2024 18:43:06 -0500 Subject: [PATCH 10/26] refactor(NotificationUtils): Extract method to get trip email subject. --- .../controllers/api/MonitoredTripController.java | 2 +- .../middleware/tripmonitor/jobs/CheckMonitoredTrip.java | 4 +--- .../middleware/utils/NotificationUtils.java | 8 ++++++++ 3 files changed, 10 insertions(+), 4 deletions(-) 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 ad1559a2a..7d2cf4349 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java @@ -199,7 +199,7 @@ private void notifyCompanion(MonitoredTrip monitoredTrip, OtpUser companionUser, NotificationUtils.sendEmail( companionUser, // TODO: Set the sender as the user who added you to their trip (like GitHub, Jira, etc) - monitoredTrip.tripName + " Notification", // TODO: Reuse trip monitor notification subject + NotificationUtils.getTripEmailSubject(companionUser, locale, monitoredTrip), "ShareTripText.ftl", // TODO: See if msg body can be reused "ShareTripHtml.ftl", Map.of( diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java index a2be9347e..0b0ad3328 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java @@ -586,9 +586,7 @@ private boolean sendPush(OtpUser otpUser, Map data) { */ private boolean sendEmail(OtpUser otpUser, Map data) { Locale locale = getOtpUserLocale(); - String subject = trip.tripName != null - ? String.format(Message.TRIP_EMAIL_SUBJECT.get(locale), trip.tripName) - : String.format(Message.TRIP_EMAIL_SUBJECT_FOR_USER.get(locale), otpUser.email); + String subject = NotificationUtils.getTripEmailSubject(otpUser, locale, trip); return NotificationUtils.sendEmail( otpUser, subject, diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index df51d635e..4fe83e94b 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -32,6 +32,8 @@ import java.util.stream.Collectors; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.opentripplanner.middleware.i18n.Message.TRIP_EMAIL_SUBJECT; +import static org.opentripplanner.middleware.i18n.Message.TRIP_EMAIL_SUBJECT_FOR_USER; import static org.opentripplanner.middleware.i18n.Message.TRIP_SURVEY_NOTIFICATION; import static org.opentripplanner.middleware.utils.ConfigUtils.getConfigPropertyAsText; @@ -437,6 +439,12 @@ public static void updatePushDevices(OtpUser otpUser) { } } + public static String getTripEmailSubject(OtpUser otpUser, Locale locale, MonitoredTrip trip) { + return trip.tripName != null + ? String.format(TRIP_EMAIL_SUBJECT.get(locale), trip.tripName) + : String.format(TRIP_EMAIL_SUBJECT_FOR_USER.get(locale), otpUser.email); + } + static class NotificationInfo { /** ID for tracking notifications and survey responses. */ public final String notificationId; From 615caad6b484fdfadc8ee86100f897b18a02c2ba Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 26 Nov 2024 11:09:05 -0500 Subject: [PATCH 11/26] docs(swagger): Update snapshot. --- src/main/resources/latest-spark-swagger-output.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/resources/latest-spark-swagger-output.yaml b/src/main/resources/latest-spark-swagger-output.yaml index 60ecf59fd..ad67eafb7 100644 --- a/src/main/resources/latest-spark-swagger-output.yaml +++ b/src/main/resources/latest-spark-swagger-output.yaml @@ -2589,6 +2589,8 @@ definitions: MonitoredTrip: type: "object" properties: + TRIPS_PATH: + type: "string" userId: type: "string" tripName: From 34aed525660c61f747c0af79632bc6f1d88e2587 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 26 Nov 2024 11:10:37 -0500 Subject: [PATCH 12/26] refactor(NotificationUtils): Move companion notification code to NotificationUtils. --- .../api/MonitoredTripController.java | 68 +----------------- .../middleware/utils/NotificationUtils.java | 69 +++++++++++++++++++ .../utils/NotificationUtilsTest.java | 22 ++++++ 3 files changed, 94 insertions(+), 65 deletions(-) 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 7d2cf4349..7667c60e0 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java @@ -5,7 +5,6 @@ import com.mongodb.client.model.Filters; import org.bson.conversions.Bson; import org.eclipse.jetty.http.HttpStatus; -import org.opentripplanner.middleware.i18n.Message; import org.opentripplanner.middleware.models.ItineraryExistence; import org.opentripplanner.middleware.models.MonitoredTrip; import org.opentripplanner.middleware.models.OtpUser; @@ -13,7 +12,6 @@ import org.opentripplanner.middleware.models.RelatedUser; import org.opentripplanner.middleware.tripmonitor.jobs.CheckMonitoredTrip; import org.opentripplanner.middleware.tripmonitor.jobs.MonitoredTripLocks; -import org.opentripplanner.middleware.utils.ConfigUtils; import org.opentripplanner.middleware.utils.InvalidItineraryReason; import org.opentripplanner.middleware.utils.JsonUtils; import org.opentripplanner.middleware.utils.NotificationUtils; @@ -21,8 +19,6 @@ import spark.Request; import spark.Response; -import java.util.Locale; -import java.util.Map; import java.util.Set; import java.util.stream.Collectors; @@ -30,11 +26,8 @@ import static com.mongodb.client.model.Filters.eq; import static org.opentripplanner.middleware.models.MonitoredTrip.USER_ID_FIELD_NAME; import static org.opentripplanner.middleware.models.MonitoredTrip.getAddedUsers; -import static org.opentripplanner.middleware.tripmonitor.jobs.CheckMonitoredTrip.SETTINGS_PATH; import static org.opentripplanner.middleware.utils.ConfigUtils.getConfigPropertyAsInt; import static org.opentripplanner.middleware.utils.HttpUtils.JSON_ONLY; -import static org.opentripplanner.middleware.utils.I18nUtils.getOtpUserLocale; -import static org.opentripplanner.middleware.utils.I18nUtils.label; import static org.opentripplanner.middleware.utils.JsonUtils.getPOJOFromRequestBody; import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt; @@ -46,16 +39,6 @@ public class MonitoredTripController extends ApiController { private static final int MAXIMUM_PERMITTED_MONITORED_TRIPS = getConfigPropertyAsInt("MAXIMUM_PERMITTED_MONITORED_TRIPS", 5); - private final String OTP_UI_NAME = ConfigUtils.getConfigPropertyAsText("OTP_UI_NAME"); - - private static final String OTP_UI_URL = ConfigUtils.getConfigPropertyAsText("OTP_UI_URL"); - - private enum UserType { - COMPANION, - OBSERVER, - PRIMARY_TRAVELER - } - public MonitoredTripController(String apiPrefix) { super(apiPrefix, Persistence.monitoredTrips, "secure/monitoredtrip"); } @@ -156,65 +139,20 @@ private void notifyTripCompanionsAndObservers(MonitoredTrip monitoredTrip, Monit if (usersToNotify.companion != null) { OtpUser companionUser = Persistence.otpUsers.getOneFiltered(Filters.eq("email", usersToNotify.companion.email)); - notifyCompanion(monitoredTrip, companionUser, UserType.COMPANION); + NotificationUtils.notifyCompanion(monitoredTrip, companionUser, NotificationUtils.UserType.COMPANION); } if (usersToNotify.primary != null) { // email could be used too for primary users OtpUser primaryUser = Persistence.otpUsers.getById(usersToNotify.primary.userId); - notifyCompanion(monitoredTrip, primaryUser, UserType.PRIMARY_TRAVELER); + NotificationUtils.notifyCompanion(monitoredTrip, primaryUser, NotificationUtils.UserType.PRIMARY_TRAVELER); } if (!usersToNotify.observers.isEmpty()) { for (RelatedUser observer : usersToNotify.observers) { OtpUser observerUser = Persistence.otpUsers.getOneFiltered(Filters.eq("email", observer.email)); - notifyCompanion(monitoredTrip, observerUser, UserType.OBSERVER); - } - } - } - - private void notifyCompanion(MonitoredTrip monitoredTrip, OtpUser companionUser, UserType userType) { - if (companionUser != null) { - Locale locale = getOtpUserLocale(companionUser); - String tripLinkLabel = Message.TRIP_LINK_TEXT.get(locale); - String tripUrl = monitoredTrip.getTripUrl(); - - OtpUser tripCreator = Persistence.otpUsers.getById(monitoredTrip.userId); - - String greeting; - switch (userType) { - case COMPANION: - greeting = "%s added you as a companion on their trip:"; - break; - case PRIMARY_TRAVELER: - greeting = "%s made you the primary traveler on this trip:"; - break; - case OBSERVER: - default: - greeting = "%s added you as an observer for their trip:"; - break; + NotificationUtils.notifyCompanion(monitoredTrip, observerUser, NotificationUtils.UserType.OBSERVER); } - - // TODO: finish i18n - NotificationUtils.sendEmail( - companionUser, - // TODO: Set the sender as the user who added you to their trip (like GitHub, Jira, etc) - NotificationUtils.getTripEmailSubject(companionUser, locale, monitoredTrip), - "ShareTripText.ftl", // TODO: See if msg body can be reused - "ShareTripHtml.ftl", - Map.of( - "emailGreeting", String.format( - greeting, - tripCreator.email - ), - "tripUrl", tripUrl, - "tripLinkAnchorLabel", tripLinkLabel, - "tripLinkLabelAndUrl", label(tripLinkLabel, tripUrl, locale), - "emailFooter", String.format(Message.TRIP_EMAIL_FOOTER.get(locale), OTP_UI_NAME), - "manageLinkText", Message.TRIP_EMAIL_MANAGE_NOTIFICATIONS.get(locale), - "manageLinkUrl", String.format("%s%s", OTP_UI_URL, SETTINGS_PATH) - ) - ); } } diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index 4fe83e94b..d38a6b1ef 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -35,7 +35,10 @@ import static org.opentripplanner.middleware.i18n.Message.TRIP_EMAIL_SUBJECT; import static org.opentripplanner.middleware.i18n.Message.TRIP_EMAIL_SUBJECT_FOR_USER; import static org.opentripplanner.middleware.i18n.Message.TRIP_SURVEY_NOTIFICATION; +import static org.opentripplanner.middleware.tripmonitor.jobs.CheckMonitoredTrip.SETTINGS_PATH; import static org.opentripplanner.middleware.utils.ConfigUtils.getConfigPropertyAsText; +import static org.opentripplanner.middleware.utils.I18nUtils.getOtpUserLocale; +import static org.opentripplanner.middleware.utils.I18nUtils.label; /** * This class contains utils for sending SMS, email, and push notifications. @@ -58,6 +61,14 @@ public class NotificationUtils { 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"); + private static final String OTP_UI_NAME = ConfigUtils.getConfigPropertyAsText("OTP_UI_NAME"); + private static final String OTP_UI_URL = ConfigUtils.getConfigPropertyAsText("OTP_UI_URL"); + + public enum UserType { + COMPANION, + OBSERVER, + PRIMARY_TRAVELER + } /** * Although SMS are 160 characters long and Twilio supports sending up to 1600 characters, @@ -439,12 +450,70 @@ public static void updatePushDevices(OtpUser otpUser) { } } + /** + * Gets the localized subject line for a trip notification. + */ public static String getTripEmailSubject(OtpUser otpUser, Locale locale, MonitoredTrip trip) { return trip.tripName != null ? String.format(TRIP_EMAIL_SUBJECT.get(locale), trip.tripName) : String.format(TRIP_EMAIL_SUBJECT_FOR_USER.get(locale), otpUser.email); } + /** + * Replaces the sender display name with the specified user's name (fallback to the user's email). + */ + public static String replaceUserNameInFromEmail(String fromEmail, OtpUser otpUser) { + int firstBracketIndex = fromEmail.indexOf('<'); + int lastBracketIndex = fromEmail.indexOf('>'); + String displayedName = Strings.isBlank(otpUser.name) ? otpUser.email : otpUser.name; + return String.format("%s %s", displayedName, fromEmail.substring(firstBracketIndex, lastBracketIndex + 1)); + } + + public static void notifyCompanion(MonitoredTrip monitoredTrip, OtpUser companionUser, UserType userType) { + if (companionUser != null) { + Locale locale = getOtpUserLocale(companionUser); + String tripLinkLabel = org.opentripplanner.middleware.i18n.Message.TRIP_LINK_TEXT.get(locale); + String tripUrl = monitoredTrip.getTripUrl(); + + OtpUser tripCreator = Persistence.otpUsers.getById(monitoredTrip.userId); + + String greeting; + switch (userType) { + case COMPANION: + greeting = "%s added you as a companion on their trip:"; + break; + case PRIMARY_TRAVELER: + greeting = "%s made you the primary traveler on this trip:"; + break; + case OBSERVER: + default: + greeting = "%s added you as an observer for their trip:"; + break; + } + + // TODO: finish i18n + sendEmail( + replaceUserNameInFromEmail(FROM_EMAIL, Persistence.otpUsers.getById(monitoredTrip.userId)), + companionUser.email, + getTripEmailSubject(companionUser, locale, monitoredTrip), + "ShareTripText.ftl", // TODO: See if msg body can be reused + "ShareTripHtml.ftl", + Map.of( + "emailGreeting", String.format( + greeting, + tripCreator.email + ), + "tripUrl", tripUrl, + "tripLinkAnchorLabel", tripLinkLabel, + "tripLinkLabelAndUrl", label(tripLinkLabel, tripUrl, locale), + "emailFooter", String.format(org.opentripplanner.middleware.i18n.Message.TRIP_EMAIL_FOOTER.get(locale), OTP_UI_NAME), + "manageLinkText", org.opentripplanner.middleware.i18n.Message.TRIP_EMAIL_MANAGE_NOTIFICATIONS.get(locale), + "manageLinkUrl", String.format("%s%s", OTP_UI_URL, SETTINGS_PATH) + ) + ); + } + } + static class NotificationInfo { /** ID for tracking notifications and survey responses. */ public final String notificationId; diff --git a/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java b/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java index 22f6b546e..d8336ea8d 100644 --- a/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java +++ b/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java @@ -142,4 +142,26 @@ static Stream createNotificationInfoCases() { ) ); } + + @ParameterizedTest + @MethodSource("createCanReplaceUserNameInFromEmailCases") + void canReplaceUserNameInFromEmail(OtpUser user, String expectedPrefix) { + final String emailAlias = ""; + String configuredFromEmail = String.format("RideGuide %s", emailAlias); + assertEquals(String.format("%s %s", expectedPrefix, emailAlias), NotificationUtils.replaceUserNameInFromEmail(configuredFromEmail, user)); + } + + private static Stream createCanReplaceUserNameInFromEmailCases() { + OtpUser user1 = new OtpUser(); + user1.email = "user1@example.com"; + + OtpUser user2 = new OtpUser(); + user2.email = "user2@example.com"; + user2.name = "Joan Smith"; + + return Stream.of( + Arguments.of(user1, user1.email), + Arguments.of(user2, user2.name) + ); + } } From 1f951044b5760d88cd6e37230228d4fa1023bd83 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 26 Nov 2024 12:39:53 -0500 Subject: [PATCH 13/26] refactor(NotificationUtils): Add i18n to messages, add more tests. --- .../middleware/i18n/Message.java | 3 +++ .../middleware/utils/NotificationUtils.java | 26 +++++++++++++------ src/main/resources/Message.properties | 3 +++ src/main/resources/Message_fr.properties | 3 +++ .../utils/NotificationUtilsTest.java | 15 ++++++----- 5 files changed, 36 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/i18n/Message.java b/src/main/java/org/opentripplanner/middleware/i18n/Message.java index 1870b5dc1..5449885b2 100644 --- a/src/main/java/org/opentripplanner/middleware/i18n/Message.java +++ b/src/main/java/org/opentripplanner/middleware/i18n/Message.java @@ -44,6 +44,9 @@ public enum Message { TRIP_DELAY_EARLY, TRIP_DELAY_LATE, TRIP_DELAY_MINUTES, + TRIP_INVITE_COMPANION, + TRIP_INVITE_PRIMARY_TRAVELER, + TRIP_INVITE_OBSERVER, TRIP_NOT_FOUND_NOTIFICATION, TRIP_NO_LONGER_POSSIBLE_NOTIFICATION, TRIP_REMINDER_NOTIFICATION, diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index d38a6b1ef..d9cf15640 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -32,8 +32,14 @@ import java.util.stream.Collectors; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.opentripplanner.middleware.i18n.Message.TRIP_EMAIL_FOOTER; +import static org.opentripplanner.middleware.i18n.Message.TRIP_EMAIL_MANAGE_NOTIFICATIONS; import static org.opentripplanner.middleware.i18n.Message.TRIP_EMAIL_SUBJECT; import static org.opentripplanner.middleware.i18n.Message.TRIP_EMAIL_SUBJECT_FOR_USER; +import static org.opentripplanner.middleware.i18n.Message.TRIP_INVITE_COMPANION; +import static org.opentripplanner.middleware.i18n.Message.TRIP_INVITE_OBSERVER; +import static org.opentripplanner.middleware.i18n.Message.TRIP_INVITE_PRIMARY_TRAVELER; +import static org.opentripplanner.middleware.i18n.Message.TRIP_LINK_TEXT; import static org.opentripplanner.middleware.i18n.Message.TRIP_SURVEY_NOTIFICATION; import static org.opentripplanner.middleware.tripmonitor.jobs.CheckMonitoredTrip.SETTINGS_PATH; import static org.opentripplanner.middleware.utils.ConfigUtils.getConfigPropertyAsText; @@ -463,16 +469,21 @@ public static String getTripEmailSubject(OtpUser otpUser, Locale locale, Monitor * Replaces the sender display name with the specified user's name (fallback to the user's email). */ public static String replaceUserNameInFromEmail(String fromEmail, OtpUser otpUser) { + if (Strings.isBlank(fromEmail)) return fromEmail; + int firstBracketIndex = fromEmail.indexOf('<'); int lastBracketIndex = fromEmail.indexOf('>'); String displayedName = Strings.isBlank(otpUser.name) ? otpUser.email : otpUser.name; return String.format("%s %s", displayedName, fromEmail.substring(firstBracketIndex, lastBracketIndex + 1)); } + /** + * Sends a notification to a specified companion user. + */ public static void notifyCompanion(MonitoredTrip monitoredTrip, OtpUser companionUser, UserType userType) { if (companionUser != null) { Locale locale = getOtpUserLocale(companionUser); - String tripLinkLabel = org.opentripplanner.middleware.i18n.Message.TRIP_LINK_TEXT.get(locale); + String tripLinkLabel = TRIP_LINK_TEXT.get(locale); String tripUrl = monitoredTrip.getTripUrl(); OtpUser tripCreator = Persistence.otpUsers.getById(monitoredTrip.userId); @@ -480,20 +491,19 @@ public static void notifyCompanion(MonitoredTrip monitoredTrip, OtpUser companio String greeting; switch (userType) { case COMPANION: - greeting = "%s added you as a companion on their trip:"; + greeting = TRIP_INVITE_COMPANION.get(locale); break; case PRIMARY_TRAVELER: - greeting = "%s made you the primary traveler on this trip:"; + greeting = TRIP_INVITE_PRIMARY_TRAVELER.get(locale); break; case OBSERVER: default: - greeting = "%s added you as an observer for their trip:"; + greeting = TRIP_INVITE_OBSERVER.get(locale); break; } - // TODO: finish i18n sendEmail( - replaceUserNameInFromEmail(FROM_EMAIL, Persistence.otpUsers.getById(monitoredTrip.userId)), + replaceUserNameInFromEmail(FROM_EMAIL, tripCreator), companionUser.email, getTripEmailSubject(companionUser, locale, monitoredTrip), "ShareTripText.ftl", // TODO: See if msg body can be reused @@ -506,8 +516,8 @@ public static void notifyCompanion(MonitoredTrip monitoredTrip, OtpUser companio "tripUrl", tripUrl, "tripLinkAnchorLabel", tripLinkLabel, "tripLinkLabelAndUrl", label(tripLinkLabel, tripUrl, locale), - "emailFooter", String.format(org.opentripplanner.middleware.i18n.Message.TRIP_EMAIL_FOOTER.get(locale), OTP_UI_NAME), - "manageLinkText", org.opentripplanner.middleware.i18n.Message.TRIP_EMAIL_MANAGE_NOTIFICATIONS.get(locale), + "emailFooter", String.format(TRIP_EMAIL_FOOTER.get(locale), OTP_UI_NAME), + "manageLinkText", TRIP_EMAIL_MANAGE_NOTIFICATIONS.get(locale), "manageLinkUrl", String.format("%s%s", OTP_UI_URL, SETTINGS_PATH) ) ); diff --git a/src/main/resources/Message.properties b/src/main/resources/Message.properties index d0b090e68..397aae44b 100644 --- a/src/main/resources/Message.properties +++ b/src/main/resources/Message.properties @@ -29,6 +29,9 @@ TRIP_DELAY_ON_TIME = about on time TRIP_DELAY_EARLY = %s early TRIP_DELAY_LATE = %s late TRIP_DELAY_MINUTES = %d minutes +TRIP_INVITE_COMPANION = %s added you as a companion for their trip. +TRIP_INVITE_PRIMARY_TRAVELER = %s made you the primary traveler on this trip. +TRIP_INVITE_OBSERVER = %s added you as an observer for their trip. 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. diff --git a/src/main/resources/Message_fr.properties b/src/main/resources/Message_fr.properties index cc147ec24..b0a9ade34 100644 --- a/src/main/resources/Message_fr.properties +++ b/src/main/resources/Message_fr.properties @@ -29,6 +29,9 @@ TRIP_DELAY_ON_TIME = TRIP_DELAY_EARLY = %s en avance TRIP_DELAY_LATE = %s en retard TRIP_DELAY_MINUTES = %d minutes +TRIP_INVITE_COMPANION = %s vous a ajouté comme accompagnateur.trice pour un trajet. +TRIP_INVITE_PRIMARY_TRAVELER = %s vous a fait le voyageur principal sur ce trajet. +TRIP_INVITE_OBSERVER = %s vous a ajouté comme observateur.trice pour un trajet. 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. diff --git a/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java b/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java index d8336ea8d..c59617072 100644 --- a/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java +++ b/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java @@ -145,10 +145,10 @@ static Stream createNotificationInfoCases() { @ParameterizedTest @MethodSource("createCanReplaceUserNameInFromEmailCases") - void canReplaceUserNameInFromEmail(OtpUser user, String expectedPrefix) { - final String emailAlias = ""; - String configuredFromEmail = String.format("RideGuide %s", emailAlias); - assertEquals(String.format("%s %s", expectedPrefix, emailAlias), NotificationUtils.replaceUserNameInFromEmail(configuredFromEmail, user)); + void canReplaceUserNameInFromEmail(String emailAlias, OtpUser user, String expectedPrefix) { + String fromEmail = emailAlias != null ? String.format("RideGuide %s", emailAlias) : null; + String expected = fromEmail != null ? fromEmail.replace("RideGuide", expectedPrefix) : null; + assertEquals(expected, NotificationUtils.replaceUserNameInFromEmail(fromEmail, user)); } private static Stream createCanReplaceUserNameInFromEmailCases() { @@ -159,9 +159,12 @@ private static Stream createCanReplaceUserNameInFromEmailCases() { user2.email = "user2@example.com"; user2.name = "Joan Smith"; + String emailAlias = ""; + return Stream.of( - Arguments.of(user1, user1.email), - Arguments.of(user2, user2.name) + Arguments.of(emailAlias, user1, user1.email), + Arguments.of(null, user1, user1.email), + Arguments.of(emailAlias, user2, user2.name) ); } } From a6b9c4095139357a27f8e7e0d1c601356bda4188 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 26 Nov 2024 13:11:08 -0500 Subject: [PATCH 14/26] perf(NotificationUtils): Extract trip creator before sending various notifications. --- .../controllers/api/MonitoredTripController.java | 7 ++++--- .../middleware/utils/NotificationUtils.java | 6 ++---- 2 files changed, 6 insertions(+), 7 deletions(-) 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 7667c60e0..b6326cc74 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java @@ -136,22 +136,23 @@ private void preCreateOrUpdateChecks(MonitoredTrip monitoredTrip, Request req) { /** Notify users added as companions or observers to a trip. (Removed users won't get notified.) */ private void notifyTripCompanionsAndObservers(MonitoredTrip monitoredTrip, MonitoredTrip originalTrip) { MonitoredTrip.TripUsers usersToNotify = getAddedUsers(monitoredTrip, originalTrip); + OtpUser tripCreator = Persistence.otpUsers.getById(monitoredTrip.userId); if (usersToNotify.companion != null) { OtpUser companionUser = Persistence.otpUsers.getOneFiltered(Filters.eq("email", usersToNotify.companion.email)); - NotificationUtils.notifyCompanion(monitoredTrip, companionUser, NotificationUtils.UserType.COMPANION); + NotificationUtils.notifyCompanion(monitoredTrip, tripCreator, companionUser, NotificationUtils.UserType.COMPANION); } if (usersToNotify.primary != null) { // email could be used too for primary users OtpUser primaryUser = Persistence.otpUsers.getById(usersToNotify.primary.userId); - NotificationUtils.notifyCompanion(monitoredTrip, primaryUser, NotificationUtils.UserType.PRIMARY_TRAVELER); + NotificationUtils.notifyCompanion(monitoredTrip, tripCreator, primaryUser, NotificationUtils.UserType.PRIMARY_TRAVELER); } if (!usersToNotify.observers.isEmpty()) { for (RelatedUser observer : usersToNotify.observers) { OtpUser observerUser = Persistence.otpUsers.getOneFiltered(Filters.eq("email", observer.email)); - NotificationUtils.notifyCompanion(monitoredTrip, observerUser, NotificationUtils.UserType.OBSERVER); + NotificationUtils.notifyCompanion(monitoredTrip, tripCreator, observerUser, NotificationUtils.UserType.OBSERVER); } } } diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index d9cf15640..772d2d558 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -480,14 +480,12 @@ public static String replaceUserNameInFromEmail(String fromEmail, OtpUser otpUse /** * Sends a notification to a specified companion user. */ - public static void notifyCompanion(MonitoredTrip monitoredTrip, OtpUser companionUser, UserType userType) { + public static void notifyCompanion(MonitoredTrip monitoredTrip, OtpUser tripCreator, OtpUser companionUser, UserType userType) { if (companionUser != null) { Locale locale = getOtpUserLocale(companionUser); String tripLinkLabel = TRIP_LINK_TEXT.get(locale); String tripUrl = monitoredTrip.getTripUrl(); - OtpUser tripCreator = Persistence.otpUsers.getById(monitoredTrip.userId); - String greeting; switch (userType) { case COMPANION: @@ -506,7 +504,7 @@ public static void notifyCompanion(MonitoredTrip monitoredTrip, OtpUser companio replaceUserNameInFromEmail(FROM_EMAIL, tripCreator), companionUser.email, getTripEmailSubject(companionUser, locale, monitoredTrip), - "ShareTripText.ftl", // TODO: See if msg body can be reused + "ShareTripText.ftl", "ShareTripHtml.ftl", Map.of( "emailGreeting", String.format( From 2a64b884c645b09a8c0ad357bce70a6e81ef097f Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 26 Nov 2024 14:04:03 -0500 Subject: [PATCH 15/26] refactor(NotificationUtils): Extract trip email attributes. --- .../middleware/models/MonitoredTrip.java | 13 ------- .../tripmonitor/jobs/CheckMonitoredTrip.java | 12 ++---- .../middleware/utils/NotificationUtils.java | 37 ++++++++++++------- 3 files changed, 26 insertions(+), 36 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java index 39f4e31ba..216adf5f7 100644 --- a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java @@ -17,7 +17,6 @@ import org.opentripplanner.middleware.persistence.Persistence; import org.opentripplanner.middleware.persistence.TypedPersistence; import org.opentripplanner.middleware.tripmonitor.JourneyState; -import org.opentripplanner.middleware.utils.ConfigUtils; import org.opentripplanner.middleware.utils.DateTimeUtils; import org.opentripplanner.middleware.utils.ItineraryUtils; import spark.Request; @@ -32,8 +31,6 @@ import java.util.function.Function; import java.util.stream.Collectors; -import static org.opentripplanner.middleware.tripmonitor.jobs.CheckMonitoredTrip.ACCOUNT_PATH; - /** * A monitored trip represents a trip a user would like to receive notification on if affected by a delay and/or route * change. @@ -43,10 +40,6 @@ public class MonitoredTrip extends Model { public static final String USER_ID_FIELD_NAME = "userId"; - private final String TRIPS_PATH = ACCOUNT_PATH + "/trips"; - - private static final String OTP_UI_URL = ConfigUtils.getConfigPropertyAsText("OTP_UI_URL"); - /** * Mongo Id of the {@link OtpUser} who owns this monitored trip. */ @@ -437,12 +430,6 @@ public boolean isOneTime() { return !monday && !tuesday && !wednesday && !thursday && !friday && !saturday && !sunday; } - @JsonIgnore - @BsonIgnore - public String getTripUrl() { - return String.format("%s%s/%s", OTP_UI_URL, TRIPS_PATH, id); - } - /** * Gets users not previously involved (as primary traveler, companion, or observer) in a trip. */ diff --git a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java index 0b0ad3328..54e45a41e 100644 --- a/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java @@ -527,21 +527,15 @@ private void sendNotifications() { String tripNameOrReminder = hasInitialReminder ? initialReminderNotification.body : trip.tripName; Locale locale = getOtpUserLocale(); - String tripLinkLabel = Message.TRIP_LINK_TEXT.get(locale); - String tripUrl = trip.getTripUrl(); // A HashMap is needed instead of a Map for template data to be serialized to the template renderer. - Map templateData = new HashMap<>(Map.of( + Map templateData = new HashMap<>(); + templateData.putAll(Map.of( "emailGreeting", Message.TRIP_EMAIL_GREETING.get(locale), "tripNameOrReminder", tripNameOrReminder, - "tripLinkLabelAndUrl", label(tripLinkLabel, tripUrl, locale), - "tripLinkAnchorLabel", tripLinkLabel, - "tripUrl", tripUrl, - "emailFooter", String.format(Message.TRIP_EMAIL_FOOTER.get(locale), OTP_UI_NAME), - "manageLinkText", Message.TRIP_EMAIL_MANAGE_NOTIFICATIONS.get(locale), - "manageLinkUrl", String.format("%s%s", OTP_UI_URL, SETTINGS_PATH), "notifications", new ArrayList<>(notifications), "smsFooter", Message.SMS_STOP_NOTIFICATIONS.get(locale) )); + templateData.putAll(NotificationUtils.getTripNotificationFields(trip, locale)); if (hasInitialReminder) { templateData.put("initialReminder", initialReminderNotification); } diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index 772d2d558..f82276355 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -25,6 +25,7 @@ import java.io.IOException; import java.net.URI; import java.net.URLEncoder; +import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -41,6 +42,7 @@ import static org.opentripplanner.middleware.i18n.Message.TRIP_INVITE_PRIMARY_TRAVELER; import static org.opentripplanner.middleware.i18n.Message.TRIP_LINK_TEXT; import static org.opentripplanner.middleware.i18n.Message.TRIP_SURVEY_NOTIFICATION; +import static org.opentripplanner.middleware.tripmonitor.jobs.CheckMonitoredTrip.ACCOUNT_PATH; import static org.opentripplanner.middleware.tripmonitor.jobs.CheckMonitoredTrip.SETTINGS_PATH; import static org.opentripplanner.middleware.utils.ConfigUtils.getConfigPropertyAsText; import static org.opentripplanner.middleware.utils.I18nUtils.getOtpUserLocale; @@ -69,6 +71,7 @@ public class NotificationUtils { private static final String TRIP_SURVEY_SUBDOMAIN = getConfigPropertyAsText("TRIP_SURVEY_SUBDOMAIN"); private static final String OTP_UI_NAME = ConfigUtils.getConfigPropertyAsText("OTP_UI_NAME"); private static final String OTP_UI_URL = ConfigUtils.getConfigPropertyAsText("OTP_UI_URL"); + private static final String TRIPS_PATH = ACCOUNT_PATH + "/trips"; public enum UserType { COMPANION, @@ -483,8 +486,6 @@ public static String replaceUserNameInFromEmail(String fromEmail, OtpUser otpUse public static void notifyCompanion(MonitoredTrip monitoredTrip, OtpUser tripCreator, OtpUser companionUser, UserType userType) { if (companionUser != null) { Locale locale = getOtpUserLocale(companionUser); - String tripLinkLabel = TRIP_LINK_TEXT.get(locale); - String tripUrl = monitoredTrip.getTripUrl(); String greeting; switch (userType) { @@ -500,28 +501,36 @@ public static void notifyCompanion(MonitoredTrip monitoredTrip, OtpUser tripCrea break; } + // A HashMap is needed instead of a Map for template data to be serialized to the template renderer. + Map templateData = new HashMap<>(); + templateData.put("emailGreeting", String.format(greeting, tripCreator.email)); + templateData.putAll(getTripNotificationFields(monitoredTrip, locale)); + sendEmail( replaceUserNameInFromEmail(FROM_EMAIL, tripCreator), companionUser.email, getTripEmailSubject(companionUser, locale, monitoredTrip), "ShareTripText.ftl", "ShareTripHtml.ftl", - Map.of( - "emailGreeting", String.format( - greeting, - tripCreator.email - ), - "tripUrl", tripUrl, - "tripLinkAnchorLabel", tripLinkLabel, - "tripLinkLabelAndUrl", label(tripLinkLabel, tripUrl, locale), - "emailFooter", String.format(TRIP_EMAIL_FOOTER.get(locale), OTP_UI_NAME), - "manageLinkText", TRIP_EMAIL_MANAGE_NOTIFICATIONS.get(locale), - "manageLinkUrl", String.format("%s%s", OTP_UI_URL, SETTINGS_PATH) - ) + templateData ); } } + public static Map getTripNotificationFields(MonitoredTrip monitoredTrip, Locale locale) { + String tripLinkLabel = TRIP_LINK_TEXT.get(locale); + String tripUrl = String.format("%s%s/%s", OTP_UI_URL, TRIPS_PATH, monitoredTrip.id); + + return Map.of( + "tripUrl", tripUrl, + "tripLinkAnchorLabel", tripLinkLabel, + "tripLinkLabelAndUrl", label(tripLinkLabel, tripUrl, locale), + "emailFooter", String.format(TRIP_EMAIL_FOOTER.get(locale), OTP_UI_NAME), + "manageLinkText", TRIP_EMAIL_MANAGE_NOTIFICATIONS.get(locale), + "manageLinkUrl", String.format("%s%s", OTP_UI_URL, SETTINGS_PATH) + ); + } + static class NotificationInfo { /** ID for tracking notifications and survey responses. */ public final String notificationId; From 941f34c05211fa0442dd901e2df9ecb24bde82d2 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 26 Nov 2024 14:10:36 -0500 Subject: [PATCH 16/26] docs(swagger): Update snapshot. --- src/main/resources/latest-spark-swagger-output.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/resources/latest-spark-swagger-output.yaml b/src/main/resources/latest-spark-swagger-output.yaml index ad67eafb7..60ecf59fd 100644 --- a/src/main/resources/latest-spark-swagger-output.yaml +++ b/src/main/resources/latest-spark-swagger-output.yaml @@ -2589,8 +2589,6 @@ definitions: MonitoredTrip: type: "object" properties: - TRIPS_PATH: - type: "string" userId: type: "string" tripName: From be2f4566b8eae56a0155f5fb432852f8c5bbd040 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 26 Nov 2024 14:19:14 -0500 Subject: [PATCH 17/26] refactor(RelatedUser): Hide isConfirmed from serialization. --- .../org/opentripplanner/middleware/models/RelatedUser.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java b/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java index 47ad86f83..25b518b56 100644 --- a/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/RelatedUser.java @@ -1,5 +1,8 @@ package org.opentripplanner.middleware.models; +import com.fasterxml.jackson.annotation.JsonIgnore; +import org.bson.codecs.pojo.annotations.BsonIgnore; + /** A related user is a companion or observer requested by a dependent. */ public class RelatedUser { public enum RelatedUserStatus { @@ -30,6 +33,8 @@ public RelatedUser(String email, RelatedUserStatus status, String nickname, Stri this.acceptKey = acceptKey; } + @JsonIgnore + @BsonIgnore public boolean isConfirmed() { return status == RelatedUserStatus.CONFIRMED; } From c1aa232808de1f42f6b2ef2b1e8d37638d9c9b08 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 26 Nov 2024 14:57:56 -0500 Subject: [PATCH 18/26] fix(NotificationUtils): Edit special char if using fallback email. --- .../opentripplanner/middleware/utils/NotificationUtils.java | 4 +++- .../middleware/utils/NotificationUtilsTest.java | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index f82276355..c2fcacf90 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -476,7 +476,9 @@ public static String replaceUserNameInFromEmail(String fromEmail, OtpUser otpUse int firstBracketIndex = fromEmail.indexOf('<'); int lastBracketIndex = fromEmail.indexOf('>'); - String displayedName = Strings.isBlank(otpUser.name) ? otpUser.email : otpUser.name; + // HACK: If falling back on email, replace the "@" sign so that the user's email does not override the + // application email in brackets. + String displayedName = Strings.isBlank(otpUser.name) ? otpUser.email.replace("@", " at ") : otpUser.name; return String.format("%s %s", displayedName, fromEmail.substring(firstBracketIndex, lastBracketIndex + 1)); } diff --git a/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java b/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java index c59617072..7c09326b0 100644 --- a/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java +++ b/src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java @@ -162,8 +162,8 @@ private static Stream createCanReplaceUserNameInFromEmailCases() { String emailAlias = ""; return Stream.of( - Arguments.of(emailAlias, user1, user1.email), - Arguments.of(null, user1, user1.email), + Arguments.of(emailAlias, user1, "user1 at example.com"), + Arguments.of(null, user1, null), Arguments.of(emailAlias, user2, user2.name) ); } From e675869485fc07f81475b0cb5cc804771838b1a8 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 26 Nov 2024 16:12:54 -0500 Subject: [PATCH 19/26] refactor(NotificationUtils): Remove companion type enum. --- .../api/MonitoredTripController.java | 10 ++++-- .../middleware/utils/NotificationUtils.java | 31 +++++-------------- 2 files changed, 14 insertions(+), 27 deletions(-) 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 b6326cc74..6a5877510 100644 --- a/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java +++ b/src/main/java/org/opentripplanner/middleware/controllers/api/MonitoredTripController.java @@ -24,6 +24,9 @@ import static io.github.manusant.ss.descriptor.MethodDescriptor.path; import static com.mongodb.client.model.Filters.eq; +import static org.opentripplanner.middleware.i18n.Message.TRIP_INVITE_COMPANION; +import static org.opentripplanner.middleware.i18n.Message.TRIP_INVITE_OBSERVER; +import static org.opentripplanner.middleware.i18n.Message.TRIP_INVITE_PRIMARY_TRAVELER; import static org.opentripplanner.middleware.models.MonitoredTrip.USER_ID_FIELD_NAME; import static org.opentripplanner.middleware.models.MonitoredTrip.getAddedUsers; import static org.opentripplanner.middleware.utils.ConfigUtils.getConfigPropertyAsInt; @@ -31,6 +34,7 @@ import static org.opentripplanner.middleware.utils.JsonUtils.getPOJOFromRequestBody; import static org.opentripplanner.middleware.utils.JsonUtils.logMessageAndHalt; + /** * Implementation of the {@link ApiController} abstract class for managing {@link MonitoredTrip} entities. This * controller connects with Auth0 services using the hooks provided by {@link ApiController}. @@ -140,19 +144,19 @@ private void notifyTripCompanionsAndObservers(MonitoredTrip monitoredTrip, Monit if (usersToNotify.companion != null) { OtpUser companionUser = Persistence.otpUsers.getOneFiltered(Filters.eq("email", usersToNotify.companion.email)); - NotificationUtils.notifyCompanion(monitoredTrip, tripCreator, companionUser, NotificationUtils.UserType.COMPANION); + NotificationUtils.notifyCompanion(monitoredTrip, tripCreator, companionUser, TRIP_INVITE_COMPANION); } if (usersToNotify.primary != null) { // email could be used too for primary users OtpUser primaryUser = Persistence.otpUsers.getById(usersToNotify.primary.userId); - NotificationUtils.notifyCompanion(monitoredTrip, tripCreator, primaryUser, NotificationUtils.UserType.PRIMARY_TRAVELER); + NotificationUtils.notifyCompanion(monitoredTrip, tripCreator, primaryUser, TRIP_INVITE_PRIMARY_TRAVELER); } if (!usersToNotify.observers.isEmpty()) { for (RelatedUser observer : usersToNotify.observers) { OtpUser observerUser = Persistence.otpUsers.getOneFiltered(Filters.eq("email", observer.email)); - NotificationUtils.notifyCompanion(monitoredTrip, tripCreator, observerUser, NotificationUtils.UserType.OBSERVER); + NotificationUtils.notifyCompanion(monitoredTrip, tripCreator, observerUser, TRIP_INVITE_OBSERVER); } } } diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index c2fcacf90..600d6d7b0 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -37,9 +37,6 @@ import static org.opentripplanner.middleware.i18n.Message.TRIP_EMAIL_MANAGE_NOTIFICATIONS; import static org.opentripplanner.middleware.i18n.Message.TRIP_EMAIL_SUBJECT; import static org.opentripplanner.middleware.i18n.Message.TRIP_EMAIL_SUBJECT_FOR_USER; -import static org.opentripplanner.middleware.i18n.Message.TRIP_INVITE_COMPANION; -import static org.opentripplanner.middleware.i18n.Message.TRIP_INVITE_OBSERVER; -import static org.opentripplanner.middleware.i18n.Message.TRIP_INVITE_PRIMARY_TRAVELER; import static org.opentripplanner.middleware.i18n.Message.TRIP_LINK_TEXT; import static org.opentripplanner.middleware.i18n.Message.TRIP_SURVEY_NOTIFICATION; import static org.opentripplanner.middleware.tripmonitor.jobs.CheckMonitoredTrip.ACCOUNT_PATH; @@ -73,12 +70,6 @@ public class NotificationUtils { private static final String OTP_UI_URL = ConfigUtils.getConfigPropertyAsText("OTP_UI_URL"); private static final String TRIPS_PATH = ACCOUNT_PATH + "/trips"; - public enum UserType { - COMPANION, - OBSERVER, - PRIMARY_TRAVELER - } - /** * Although SMS are 160 characters long and Twilio supports sending up to 1600 characters, * they don't recommend sending more than 320 characters in a single "request" @@ -485,23 +476,15 @@ public static String replaceUserNameInFromEmail(String fromEmail, OtpUser otpUse /** * Sends a notification to a specified companion user. */ - public static void notifyCompanion(MonitoredTrip monitoredTrip, OtpUser tripCreator, OtpUser companionUser, UserType userType) { + public static void notifyCompanion( + MonitoredTrip monitoredTrip, + OtpUser tripCreator, + OtpUser companionUser, + org.opentripplanner.middleware.i18n.Message message + ) { if (companionUser != null) { Locale locale = getOtpUserLocale(companionUser); - - String greeting; - switch (userType) { - case COMPANION: - greeting = TRIP_INVITE_COMPANION.get(locale); - break; - case PRIMARY_TRAVELER: - greeting = TRIP_INVITE_PRIMARY_TRAVELER.get(locale); - break; - case OBSERVER: - default: - greeting = TRIP_INVITE_OBSERVER.get(locale); - break; - } + String greeting = message.get(locale); // A HashMap is needed instead of a Map for template data to be serialized to the template renderer. Map templateData = new HashMap<>(); From 4f84f69ed9d196e20c156b451a74d8e658f11612 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 26 Nov 2024 16:23:59 -0500 Subject: [PATCH 20/26] refactor: Add light touch ups. --- .../opentripplanner/middleware/models/MonitoredTrip.java | 8 ++++---- .../middleware/utils/NotificationUtils.java | 6 +++++- src/main/resources/Message_fr.properties | 4 ++-- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java index 216adf5f7..a9e67b35c 100644 --- a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java @@ -440,7 +440,7 @@ public static TripUsers getAddedUsers(MonitoredTrip monitoredTrip, MonitoredTrip originalTrip.companion == null || !originalTrip.companion.email.equals(monitoredTrip.companion.email) )) { - // notify added companion if creating trip or setting companion for the first time, + // Include the companion if creating trip or setting companion for the first time, // or setting a different companion. addedCompanion = monitoredTrip.companion; } @@ -451,7 +451,7 @@ public static TripUsers getAddedUsers(MonitoredTrip monitoredTrip, MonitoredTrip originalTrip.primary == null || !originalTrip.primary.userId.equals(monitoredTrip.primary.userId) )) { - // notify added primary traveler if creating trip or setting primary traveler for the first time, + // Include the primary traveler if creating trip or setting primary traveler for the first time, // or setting a different primary traveler. addedPrimaryTraveler = monitoredTrip.primary; } @@ -462,10 +462,10 @@ public static TripUsers getAddedUsers(MonitoredTrip monitoredTrip, MonitoredTrip .filter(RelatedUser::isConfirmed) .collect(Collectors.toList()); if (originalTrip == null || originalTrip.observers == null) { - // notify everyone + // Include all observers if creating trip or setting observers for the first time. addedObservers.addAll(confirmedObservers); } else { - // notify added observers + // If observers have been set before, include observers not previously present. Set existingObserverEmails = originalTrip.observers.stream() .map(obs -> obs.email) .collect(Collectors.toSet()); diff --git a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java index 600d6d7b0..b1635d778 100644 --- a/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java +++ b/src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java @@ -502,9 +502,13 @@ public static void notifyCompanion( } } + private static String getTripUrl(MonitoredTrip trip) { + return String.format("%s%s/%s", OTP_UI_URL, TRIPS_PATH, trip.id); + } + public static Map getTripNotificationFields(MonitoredTrip monitoredTrip, Locale locale) { String tripLinkLabel = TRIP_LINK_TEXT.get(locale); - String tripUrl = String.format("%s%s/%s", OTP_UI_URL, TRIPS_PATH, monitoredTrip.id); + String tripUrl = getTripUrl(monitoredTrip); return Map.of( "tripUrl", tripUrl, diff --git a/src/main/resources/Message_fr.properties b/src/main/resources/Message_fr.properties index b0a9ade34..1b178923d 100644 --- a/src/main/resources/Message_fr.properties +++ b/src/main/resources/Message_fr.properties @@ -29,9 +29,9 @@ TRIP_DELAY_ON_TIME = TRIP_DELAY_EARLY = %s en avance TRIP_DELAY_LATE = %s en retard TRIP_DELAY_MINUTES = %d minutes -TRIP_INVITE_COMPANION = %s vous a ajouté comme accompagnateur.trice pour un trajet. +TRIP_INVITE_COMPANION = %s vous a ajouté comme accompagnateur·trice pour un trajet. TRIP_INVITE_PRIMARY_TRAVELER = %s vous a fait le voyageur principal sur ce trajet. -TRIP_INVITE_OBSERVER = %s vous a ajouté comme observateur.trice pour un trajet. +TRIP_INVITE_OBSERVER = %s vous a ajouté comme observateur·trice pour un trajet. 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. From 01a2c75ac4cea87586acb89e6a07295329f55af0 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 4 Dec 2024 11:43:43 -0500 Subject: [PATCH 21/26] refactor(MonitoredTrip): Add predicate for confirmed companion. --- .../middleware/models/MonitoredTrip.java | 7 +++++ .../middleware/models/MonitoredTripTest.java | 29 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java index 1626ca8fd..b688282c1 100644 --- a/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java +++ b/src/main/java/org/opentripplanner/middleware/models/MonitoredTrip.java @@ -427,4 +427,11 @@ public int tripTimeMinute() { public boolean isOneTime() { return !monday && !tuesday && !wednesday && !thursday && !friday && !saturday && !sunday; } + + /** + * @return true if the trip has a (confirmed) companion, false otherwise. + */ + public boolean hasConfirmedCompanion() { + return companion != null && companion.status == RelatedUser.RelatedUserStatus.CONFIRMED; + } } diff --git a/src/test/java/org/opentripplanner/middleware/models/MonitoredTripTest.java b/src/test/java/org/opentripplanner/middleware/models/MonitoredTripTest.java index 4ba076646..dd531e8fa 100644 --- a/src/test/java/org/opentripplanner/middleware/models/MonitoredTripTest.java +++ b/src/test/java/org/opentripplanner/middleware/models/MonitoredTripTest.java @@ -1,6 +1,9 @@ package org.opentripplanner.middleware.models; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.opentripplanner.middleware.otp.OtpGraphQLTransportMode; import org.opentripplanner.middleware.otp.OtpGraphQLVariables; import org.opentripplanner.middleware.otp.response.Itinerary; @@ -41,4 +44,30 @@ void initializeFromItineraryAndQueryParamsShouldNotModifyModes() { trip.initializeFromItineraryAndQueryParams(variables); assertEquals(originalModes, trip.otp2QueryParams.modes); } + + @ParameterizedTest + @MethodSource("createHasCompanionCases") + void testHasCompanion(RelatedUser companion, boolean expected) { + MonitoredTrip ownTripWithCompanion = new MonitoredTrip(); + ownTripWithCompanion.companion = companion; + ownTripWithCompanion.userId = "trip-user-id"; + + assertEquals(expected, ownTripWithCompanion.hasConfirmedCompanion()); + } + + private static Stream createHasCompanionCases() { + RelatedUser confirmedCompanion = new RelatedUser(); + confirmedCompanion.email = "companion@example.com"; + confirmedCompanion.status = RelatedUser.RelatedUserStatus.CONFIRMED; + + RelatedUser unconfirmedCompanion = new RelatedUser(); + unconfirmedCompanion.email = "companion@example.com"; + unconfirmedCompanion.status = RelatedUser.RelatedUserStatus.INVALID; + + return Stream.of( + Arguments.of(null, false), + Arguments.of(confirmedCompanion, true), + Arguments.of(unconfirmedCompanion, false) + ); + } } From 398d7f6af0f68ccef6aafb9037b106991ad17c4e Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 4 Dec 2024 13:34:20 -0500 Subject: [PATCH 22/26] feat(UsRide...Message): Populate trusted companion field for RideGwinnett messages. --- .../triptracker/TripTrackingData.java | 3 +- ...sRideGwinnettBusOpNotificationMessage.java | 2 +- .../triptracker/NotifyBusOperatorTest.java | 54 +++++++++++++------ 3 files changed, 42 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TripTrackingData.java b/src/main/java/org/opentripplanner/middleware/triptracker/TripTrackingData.java index ae03e23b1..2e0d03abf 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/TripTrackingData.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/TripTrackingData.java @@ -28,9 +28,10 @@ public class TripTrackingData { public final TrackedJourney journey; public final List locations; - private TripTrackingData(MonitoredTrip trip, TrackedJourney journey, List locations) { + public TripTrackingData(MonitoredTrip trip, TrackedJourney journey, List locations) { this.trip = trip; this.journey = journey; + this.journey.trip = trip; this.locations = locations; } diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/interactions/busnotifiers/UsRideGwinnettBusOpNotificationMessage.java b/src/main/java/org/opentripplanner/middleware/triptracker/interactions/busnotifiers/UsRideGwinnettBusOpNotificationMessage.java index adca33576..bf93ae527 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/interactions/busnotifiers/UsRideGwinnettBusOpNotificationMessage.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/interactions/busnotifiers/UsRideGwinnettBusOpNotificationMessage.java @@ -96,7 +96,7 @@ public UsRideGwinnettBusOpNotificationMessage(Instant currentTime, TravelerPosit // 1 = Notify, 0 = Cancel. this.msg_type = 1; this.mobility_codes = getMobilityCode(travelerPosition.mobilityMode); - this.trusted_companion = false; + this.trusted_companion = travelerPosition.trackedJourney.trip.hasConfirmedCompanion(); } /** diff --git a/src/test/java/org/opentripplanner/middleware/triptracker/NotifyBusOperatorTest.java b/src/test/java/org/opentripplanner/middleware/triptracker/NotifyBusOperatorTest.java index ec81deb68..060172b66 100644 --- a/src/test/java/org/opentripplanner/middleware/triptracker/NotifyBusOperatorTest.java +++ b/src/test/java/org/opentripplanner/middleware/triptracker/NotifyBusOperatorTest.java @@ -8,7 +8,9 @@ import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; import org.opentripplanner.middleware.models.MobilityProfile; +import org.opentripplanner.middleware.models.MonitoredTrip; import org.opentripplanner.middleware.models.OtpUser; +import org.opentripplanner.middleware.models.RelatedUser; import org.opentripplanner.middleware.models.TrackedJourney; import org.opentripplanner.middleware.otp.response.Itinerary; import org.opentripplanner.middleware.otp.response.Leg; @@ -33,6 +35,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.opentripplanner.middleware.triptracker.TravelerLocator.ACCEPTABLE_AHEAD_OF_SCHEDULE_IN_MINUTES; import static org.opentripplanner.middleware.triptracker.TravelerLocator.getBusDepartureTime; @@ -46,7 +49,7 @@ class NotifyBusOperatorTest extends OtpMiddlewareTestEnvironment { private static TrackedJourney trackedJourney; - private static final String routeId = "GwinnettCountyTransit:40"; + private static final String ROUTE_ID = "GwinnettCountyTransit:40"; private static final Locale locale = Locale.US; @@ -66,7 +69,7 @@ public static void setUp() throws IOException { Itinerary.class ); UsRideGwinnettNotifyBusOperator.IS_TEST = true; - UsRideGwinnettNotifyBusOperator.US_RIDE_GWINNETT_QUALIFYING_BUS_NOTIFIER_ROUTES = List.of(routeId); + UsRideGwinnettNotifyBusOperator.US_RIDE_GWINNETT_QUALIFYING_BUS_NOTIFIER_ROUTES = List.of(ROUTE_ID); } @AfterEach @@ -86,7 +89,7 @@ void canNotifyBusOperatorForScheduledDeparture(Leg busLeg, Itinerary itinerary, String tripInstruction = TravelerLocator.getInstruction(TripStatus.ON_SCHEDULE, travelerPosition, isStartOfTrip); TripInstruction expectInstruction = new WaitForTransitInstruction(busLeg, busDepartureTime, locale); TrackedJourney updated = Persistence.trackedJourneys.getById(trackedJourney.id); - assertTrue(updated.busNotificationMessages.containsKey(routeId)); + assertTrue(updated.busNotificationMessages.containsKey(ROUTE_ID)); assertEquals(expectInstruction.build(), tripInstruction, message); } @@ -112,7 +115,7 @@ private static Stream creatNotifyBusOperatorForScheduledDepartureTrac void shouldCancelBusNotificationForStartOfTrip(boolean expected, Leg expectedLeg, Coordinates currentPosition, String message) { Leg first = firstLegBusTransit.legs.get(0); TrackedJourney journey = new TrackedJourney(); - journey.busNotificationMessages.put(routeId, "{\"msg_type\": 1}"); + journey.busNotificationMessages.put(ROUTE_ID, "{\"msg_type\": 1}"); TravelerPosition travelerPosition = new TravelerPosition(expectedLeg, journey, first, currentPosition); assertEquals(expected, ManageTripTracking.shouldCancelBusNotificationForStartOfTrip(travelerPosition), message); } @@ -161,9 +164,7 @@ void canCancelBusOperatorNotification() throws JsonProcessingException, Interrup trackedJourney = createAndPersistTrackedJourney(getEndOfWalkLegCoordinates()); TravelerPosition travelerPosition = new TravelerPosition(trackedJourney, walkToBusTransition, createOtpUser()); - busOperatorActions.handleSendNotificationAction(travelerPosition, travelerPosition.nextLeg); - TrackedJourney updated = Persistence.trackedJourneys.getById(trackedJourney.id); - assertTrue(updated.busNotificationMessages.containsKey(routeId)); + TrackedJourney updated = sendAndCheckInitialBusOperatorNotification(travelerPosition); assertEquals(1, getMessage(updated).msg_type); busOperatorActions.handleCancelNotificationAction(travelerPosition, travelerPosition.nextLeg); @@ -184,7 +185,7 @@ void canCancelBusOperatorNotification() throws JsonProcessingException, Interrup } private static UsRideGwinnettBusOpNotificationMessage getMessage(TrackedJourney updated) throws JsonProcessingException { - String messageBody = updated.busNotificationMessages.get(routeId); + String messageBody = updated.busNotificationMessages.get(ROUTE_ID); return getNotificationMessage(messageBody); } @@ -193,18 +194,41 @@ void canNotifyBusOperatorOnlyOnce() throws InterruptedException, JsonProcessingE trackedJourney = createAndPersistTrackedJourney(getEndOfWalkLegCoordinates()); TravelerPosition travelerPosition = new TravelerPosition(trackedJourney, walkToBusTransition, createOtpUser()); - busOperatorActions.handleSendNotificationAction(travelerPosition, travelerPosition.nextLeg); - TrackedJourney updated = Persistence.trackedJourneys.getById(trackedJourney.id); - assertTrue(updated.busNotificationMessages.containsKey(routeId)); - assertFalse(UsRideGwinnettNotifyBusOperator.hasNotSentNotificationForRoute(updated, routeId)); - UsRideGwinnettBusOpNotificationMessage notifyMessage = getMessage(updated); + TrackedJourney updated = sendAndCheckInitialBusOperatorNotification(travelerPosition); // A second request to notify the operator should not touch the previous request. Thread.sleep(20); busOperatorActions.handleSendNotificationAction(travelerPosition, travelerPosition.nextLeg); TrackedJourney updated2 = Persistence.trackedJourneys.getById(trackedJourney.id); - assertFalse(UsRideGwinnettNotifyBusOperator.hasNotSentNotificationForRoute(updated2, routeId)); - assertEquals(notifyMessage.timestamp, getMessage(updated2).timestamp); + assertFalse(UsRideGwinnettNotifyBusOperator.hasNotSentNotificationForRoute(updated2, ROUTE_ID)); + assertEquals(getMessage(updated).timestamp, getMessage(updated2).timestamp); + } + + private TrackedJourney sendAndCheckInitialBusOperatorNotification(TravelerPosition travelerPosition) throws JsonProcessingException { + Coordinates position = travelerPosition.currentPosition; + MonitoredTrip trip = new MonitoredTrip(); + // Add a confirmed companion to this trip + trip.companion = new RelatedUser(); + trip.companion.status = RelatedUser.RelatedUserStatus.CONFIRMED; + + // Endpoints indirectly call the TripTrackingData constructor that sets the trip field in Trackedjourney, + // so we add that here to replicate those steps. + TripTrackingData tripData = new TripTrackingData( + trip, + trackedJourney, + List.of(new TrackingLocation(travelerPosition.currentTime, position.lat, position.lon)) + ); + assertNotNull(tripData.journey.trip); + assertNotNull(trackedJourney.trip); + + busOperatorActions.handleSendNotificationAction(travelerPosition, travelerPosition.nextLeg); + + TrackedJourney updated = Persistence.trackedJourneys.getById(trackedJourney.id); + assertTrue(updated.busNotificationMessages.containsKey(ROUTE_ID)); + assertFalse(UsRideGwinnettNotifyBusOperator.hasNotSentNotificationForRoute(updated, ROUTE_ID)); + assertTrue(getMessage(updated).trusted_companion); + + return updated; } @ParameterizedTest From d9cce93e81fccf2ea81657f9063194f8db786d24 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 4 Dec 2024 14:52:26 -0500 Subject: [PATCH 23/26] fix(TrackedTripData): check for null journey before assigning trip. --- .../middleware/triptracker/TripTrackingData.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/TripTrackingData.java b/src/main/java/org/opentripplanner/middleware/triptracker/TripTrackingData.java index 2e0d03abf..685cf0761 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/TripTrackingData.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/TripTrackingData.java @@ -31,7 +31,9 @@ public class TripTrackingData { public TripTrackingData(MonitoredTrip trip, TrackedJourney journey, List locations) { this.trip = trip; this.journey = journey; - this.journey.trip = trip; + if (journey != null) { + this.journey.trip = trip; + } this.locations = locations; } From dcde5713a9971e71f9bb68c248aa1a26bc1e82cf Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 4 Dec 2024 15:00:18 -0500 Subject: [PATCH 24/26] refactor(UsRide...Message): check for null journey/trip. --- .../busnotifiers/UsRideGwinnettBusOpNotificationMessage.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/opentripplanner/middleware/triptracker/interactions/busnotifiers/UsRideGwinnettBusOpNotificationMessage.java b/src/main/java/org/opentripplanner/middleware/triptracker/interactions/busnotifiers/UsRideGwinnettBusOpNotificationMessage.java index bf93ae527..cd08d2016 100644 --- a/src/main/java/org/opentripplanner/middleware/triptracker/interactions/busnotifiers/UsRideGwinnettBusOpNotificationMessage.java +++ b/src/main/java/org/opentripplanner/middleware/triptracker/interactions/busnotifiers/UsRideGwinnettBusOpNotificationMessage.java @@ -1,5 +1,6 @@ package org.opentripplanner.middleware.triptracker.interactions.busnotifiers; +import org.opentripplanner.middleware.models.TrackedJourney; import org.opentripplanner.middleware.triptracker.TravelerPosition; import java.time.Instant; @@ -96,7 +97,8 @@ public UsRideGwinnettBusOpNotificationMessage(Instant currentTime, TravelerPosit // 1 = Notify, 0 = Cancel. this.msg_type = 1; this.mobility_codes = getMobilityCode(travelerPosition.mobilityMode); - this.trusted_companion = travelerPosition.trackedJourney.trip.hasConfirmedCompanion(); + TrackedJourney journey = travelerPosition.trackedJourney; + this.trusted_companion = journey != null && journey.trip != null && journey.trip.hasConfirmedCompanion(); } /** From 02d74f9db32244b6de46ab1d8be8d9aad86b8708 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 5 Dec 2024 10:05:30 -0500 Subject: [PATCH 25/26] test(MonitoredTripTest): Fix incorrect var assignment. --- .../opentripplanner/middleware/models/MonitoredTripTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/opentripplanner/middleware/models/MonitoredTripTest.java b/src/test/java/org/opentripplanner/middleware/models/MonitoredTripTest.java index 5063a6980..c4f89a60e 100644 --- a/src/test/java/org/opentripplanner/middleware/models/MonitoredTripTest.java +++ b/src/test/java/org/opentripplanner/middleware/models/MonitoredTripTest.java @@ -79,7 +79,7 @@ private static Stream createGetAddedUsersCases() { primary.userId = "primary-user-id"; MobilityProfileLite newPrimary = new MobilityProfileLite(); - primary.userId = "new-primary-user-id"; + newPrimary.userId = "new-primary-user-id"; RelatedUser companion = new RelatedUser("companion@example.com", CONFIRMED); RelatedUser newCompanion = new RelatedUser("new-companion@example.com", CONFIRMED); From c5dfaead5a10f4bb37c26b1412100be77dda231c Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 5 Dec 2024 15:24:48 -0500 Subject: [PATCH 26/26] fix(AbstractUser): Ignore unknown fields passed in OtpUser and other users payload. --- .../org/opentripplanner/middleware/models/AbstractUser.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/org/opentripplanner/middleware/models/AbstractUser.java b/src/main/java/org/opentripplanner/middleware/models/AbstractUser.java index 7164e3491..16ea0ab6f 100644 --- a/src/main/java/org/opentripplanner/middleware/models/AbstractUser.java +++ b/src/main/java/org/opentripplanner/middleware/models/AbstractUser.java @@ -1,6 +1,7 @@ package org.opentripplanner.middleware.models; import com.auth0.exception.Auth0Exception; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import org.opentripplanner.middleware.auth.Auth0Connection; import org.opentripplanner.middleware.auth.RequestingUser; import org.opentripplanner.middleware.auth.Permission; @@ -19,6 +20,7 @@ * It provides a place to centralize common fields that all users share (e.g., email) and common methods (such as the * authorization check {@link #canBeManagedBy}. */ +@JsonIgnoreProperties(ignoreUnknown = true) public abstract class AbstractUser extends Model { private static final Logger LOG = LoggerFactory.getLogger(AbstractUser.class); private static final long serialVersionUID = 1L;