Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OTP-1382 GMAP Push notifications for leg transition #275

Open
wants to merge 23 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
7ba414d
refactor(Initial work to check for major trip changes):
br648 Dec 3, 2024
7c466fc
refactor(Updated swagger docs and included the current leg on major c…
br648 Dec 3, 2024
21d9407
Merge branch 'dev' into feature/OTP-1382-notify-major-changes
br648 Dec 3, 2024
ee451f6
Merge branch 'dev' into feature/OTP-1382-notify-major-changes
br648 Dec 4, 2024
f9126c4
refactor(Added unit tests and made changes to wording.):
br648 Dec 4, 2024
ad42160
Merge branch 'dev' into feature/OTP-1382-notify-major-changes
br648 Dec 6, 2024
75f44c8
refactor(Various updates to monitor leg transition):
br648 Dec 9, 2024
926f994
refactor(Addressed unused params and methods. Updated Swagger docs.):
br648 Dec 9, 2024
128fb32
refactor(Fixed merge conflicts):
br648 Dec 10, 2024
91d6619
refactor(Refactor to create and process leg tranistion based notifica…
br648 Dec 10, 2024
c5582e1
refactor(Addressed PR feedback.):
br648 Dec 13, 2024
2638f04
refactor(Fixed merge conflicts and moved leg trans notify test into o…
br648 Dec 13, 2024
17363ac
refactor(Refactored otp user to provide display name):
br648 Dec 13, 2024
bc20b84
refactor(Moved various methods into the LegTransitionNotification cla…
br648 Dec 13, 2024
4e788c5
refactor(Created builder to handle basic traveler position needs):
br648 Dec 13, 2024
22ed611
refactor(Addressed PR feedback):
br648 Dec 16, 2024
5159804
refactor(Update to guard against missing target zoned date time when …
br648 Dec 16, 2024
c05e011
refactor(CheckMonitoredTrip): Update to preserve matching itinerary
br648 Dec 16, 2024
8340c43
refactor(Fixed merge conflicts):
br648 Dec 18, 2024
07ad8d8
improvement(hidden getDisplayName from Mongo and JSON serialisation):
br648 Dec 18, 2024
36b26af
chore(i18n): Add French text for new strings.
binh-dam-ibigroup Dec 18, 2024
4366df5
Merge branch 'dev' into feature/OTP-1382-notify-major-changes
br648 Dec 19, 2024
b3ab1fd
Merge branch 'feature/OTP-1382-notify-major-changes' of https://githu…
br648 Dec 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ public enum Message {
ACCEPT_DEPENDENT_EMAIL_SUBJECT,
ACCEPT_DEPENDENT_EMAIL_MANAGE,
ACCEPT_DEPENDENT_ERROR,
ARRIVED_AND_MODE_CHANGE_NOTIFICATION,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say just keep the old name MODE_CHANGE_NOTIFICATION.

ARRIVED_NOTIFICATION,
DEPARTED_NOTIFICATION,
LABEL_AND_CONTENT,
MODE_CHANGE_NOTIFICATION,
SMS_STOP_NOTIFICATIONS,
TRIP_EMAIL_SUBJECT,
TRIP_EMAIL_SUBJECT_FOR_USER,
Expand Down Expand Up @@ -51,7 +51,6 @@ public enum Message {
TRIP_INVITE_PRIMARY_TRAVELER,
TRIP_INVITE_OBSERVER,
TRIP_NAME_UNDEFINED,
TRIP_TRAVELER_GENERIC_NAME,
TRIP_NOT_FOUND_NOTIFICATION,
TRIP_NO_LONGER_POSSIBLE_NOTIFICATION,
TRIP_REMINDER_NOTIFICATION,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,32 @@
package org.opentripplanner.middleware.models;

import org.opentripplanner.middleware.i18n.Message;
import org.opentripplanner.middleware.otp.response.Leg;
import org.opentripplanner.middleware.persistence.Persistence;
import org.opentripplanner.middleware.tripmonitor.jobs.CheckMonitoredTrip;
import org.opentripplanner.middleware.tripmonitor.jobs.NotificationType;
import org.opentripplanner.middleware.triptracker.TravelerPosition;
import org.opentripplanner.middleware.triptracker.TripStatus;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.annotation.Nullable;
import java.util.ArrayList;
import java.util.List;
import java.util.HashSet;
import java.util.Locale;
import java.util.Set;

import static com.mongodb.client.model.Filters.eq;
import static org.opentripplanner.middleware.tripmonitor.jobs.NotificationType.ARRIVED_AND_MODE_CHANGE_NOTIFICATION;
import static org.opentripplanner.middleware.tripmonitor.jobs.NotificationType.ARRIVED_NOTIFICATION;
import static org.opentripplanner.middleware.tripmonitor.jobs.NotificationType.DEPARTED_NOTIFICATION;
import static org.opentripplanner.middleware.triptracker.TravelerLocator.hasRequiredTransitLeg;
import static org.opentripplanner.middleware.triptracker.TravelerLocator.hasRequiredTripStatus;
import static org.opentripplanner.middleware.triptracker.TravelerLocator.hasRequiredWalkLeg;
import static org.opentripplanner.middleware.triptracker.TravelerLocator.isApproachingEndOfLeg;
import static org.opentripplanner.middleware.triptracker.TravelerLocator.isAtStartOfLeg;

public class LegTransitionNotification {
private static final Logger LOG = LoggerFactory.getLogger(LegTransitionNotification.class);

public String travelerName;
public NotificationType notificationType;
Expand All @@ -34,28 +51,27 @@ public LegTransitionNotification(
* Create {@link TripMonitorNotification} for leg transition based on notification type.
*/
@Nullable
public TripMonitorNotification createTripMonitorNotification(NotificationType notificationType) {
private TripMonitorNotification createTripMonitorNotification(NotificationType notificationType) {
String body;
switch (notificationType) {
case MODE_CHANGE_NOTIFICATION:
case ARRIVED_AND_MODE_CHANGE_NOTIFICATION:
body = String.format(
Message.MODE_CHANGE_NOTIFICATION.get(travelerPosition.locale),
getTravelerName(),
travelerPosition.expectedLeg.mode,
travelerPosition.nextLeg.mode
Message.ARRIVED_AND_MODE_CHANGE_NOTIFICATION.get(observerLocale),
travelerName,
travelerPosition.expectedLeg.to.name
);
break;
case DEPARTED_NOTIFICATION:
body = String.format(
Message.DEPARTED_NOTIFICATION.get(observerLocale),
getTravelerName(),
travelerName,
travelerPosition.expectedLeg.from.name
);
break;
case ARRIVED_NOTIFICATION:
body = String.format(
Message.ARRIVED_NOTIFICATION.get(travelerPosition.locale),
getTravelerName(),
Message.ARRIVED_NOTIFICATION.get(observerLocale),
travelerName,
travelerPosition.expectedLeg.to.name
);
break;
Expand All @@ -66,38 +82,69 @@ public TripMonitorNotification createTripMonitorNotification(NotificationType no
}

/**
* Get the traveler's name if available, if not provide a generic traveler name.
* Get a list of users that should be notified of a traveler's leg transition.
*/
private String getTravelerName() {
if (travelerName != null) {
return travelerName;
} else {
return Message.TRIP_TRAVELER_GENERIC_NAME.get(observerLocale);
public static Set<OtpUser> getLegTransitionNotifyUsers(MonitoredTrip trip) {
Set<OtpUser> notifyUsers = new HashSet<>();

if (trip.ownedByPrimary() && trip.companion != null) {
notifyUsers.add(Persistence.otpUsers.getOneFiltered(eq("email", trip.companion.email)));
} else if (trip.ownedByCompanion() && trip.primary != null) {
notifyUsers.add(Persistence.otpUsers.getById(trip.primary.userId));
}

trip.observers.forEach(observer -> {
if (observer.isConfirmed()) {
br648 marked this conversation as resolved.
Show resolved Hide resolved
notifyUsers.add(Persistence.otpUsers.getOneFiltered(eq("email", observer.email)));
}
});

return notifyUsers;
}

/**
* Create locale specific notifications.
* If a traveler is on schedule and on either a walk or transit leg check for possible leg transition notification.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you meant to say "If a traveler is on the route (not deviated)"...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Insert comma before "check".

*/
public static TripMonitorNotification[] createLegTransitionNotifications(
List<NotificationType> legTransitionTypes,
String travelerName,
TravelerPosition travelerPosition,
Locale observerLocale
) {
List<TripMonitorNotification> tripMonitorNotifications = new ArrayList<>();
// Create locale specific notifications.
for (NotificationType legTransitionType : legTransitionTypes) {
LegTransitionNotification legTransitionNotification = new LegTransitionNotification(
travelerName,
legTransitionType,
travelerPosition,
observerLocale
);
if (legTransitionNotification.tripMonitorNotification != null) {
tripMonitorNotifications.add(legTransitionNotification.tripMonitorNotification);
public static void checkForLegTransition(TripStatus tripStatus, TravelerPosition travelerPosition, MonitoredTrip trip) {
if (
hasRequiredTripStatus(tripStatus) &&
(hasRequiredWalkLeg(travelerPosition) || hasRequiredTransitLeg(travelerPosition))
) {
NotificationType notificationType = getLegTransitionNotificationType(travelerPosition);
if (notificationType != null) {
try {
new CheckMonitoredTrip(trip).processLegTransition(notificationType, travelerPosition);
} catch (CloneNotSupportedException e) {
LOG.error("Error encountered while checking leg transition.", e);
}
}
}
return tripMonitorNotifications.toArray(new TripMonitorNotification[0]);
}
}

/**
* Depending on the traveler's proximity to the start/end of a leg return the appropriate notification type.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Insert comma before "return".

*/
private static NotificationType getLegTransitionNotificationType(TravelerPosition travelerPosition) {
if (isAtStartOfLeg(travelerPosition)) {
return DEPARTED_NOTIFICATION;
Comment on lines +132 to +133
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: Change the condition so that departing notifications are sent at the beginning of the trip only.

I found one case where I set two coordinates, one close to the end of a leg, and the next one close to the beginning of the next leg, and I got two companion notifications, one for arriving at the end of the previous leg, and one for departing the next leg, which is redundant with the previous one and is thus unneeded noise. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for consistency we leave it as is. The observer would get a notification that the traveler has arrived but not one to tell them they have departed. The next notification would be the traveler arriving at the end of the next leg. Correct?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I observed.

} else if (isApproachingEndOfLeg(travelerPosition)) {
if (hasModeChanged(travelerPosition)) {
return ARRIVED_AND_MODE_CHANGE_NOTIFICATION;
}
return ARRIVED_NOTIFICATION;
}
return null;
}

/**
* The traveler is at the end of the current leg and the mode has changed between this and the next leg.
*/
private static boolean hasModeChanged(TravelerPosition travelerPosition) {
Leg nextLeg = travelerPosition.nextLeg;
Leg expectedLeg = travelerPosition.expectedLeg;
return
isApproachingEndOfLeg(travelerPosition) &&
nextLeg != null &&
!nextLeg.mode.equalsIgnoreCase(expectedLeg.mode);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import java.util.function.Function;
import java.util.stream.Collectors;

import static com.mongodb.client.model.Filters.eq;

/**
* A monitored trip represents a trip a user would like to receive notification on if affected by a delay and/or route
* change.
Expand Down Expand Up @@ -508,4 +510,19 @@ public TripUsers(MobilityProfileLite primary, RelatedUser companion, List<Relate
this.observers = observers;
}
}

/**
* Trip created by primary user.
*/
public boolean ownedByPrimary() {
return primary != null && primary.userId.equalsIgnoreCase(userId);
}

/**
* Trip created by companion user.
*/
public boolean ownedByCompanion() {
OtpUser tripOwner = Persistence.otpUsers.getById(userId);
return tripOwner != null && companion != null && companion.email.equalsIgnoreCase(tripOwner.email);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.fasterxml.jackson.annotation.JsonGetter;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonSetter;
import org.apache.logging.log4j.util.Strings;
import org.opentripplanner.middleware.auth.Auth0Users;
import org.opentripplanner.middleware.auth.RequestingUser;
import org.opentripplanner.middleware.persistence.Persistence;
Expand Down Expand Up @@ -210,4 +211,11 @@ public Optional<TripSurveyNotification> findLastTripSurveyNotificationSent() {
if (tripSurveyNotifications == null) return Optional.empty();
return tripSurveyNotifications.stream().max(Comparator.comparingLong(n -> n.timeSent.getTime()));
}

/**
* Use name if available, if not fallback on email (which is a required field).
*/
public String getDisplayedName() {
return Strings.isBlank(name) ? email.replace("@", " at ") : name;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import java.util.Date;
import java.util.Locale;
import java.util.Objects;

/**
* Contains information about the type and details of messages to be sent to users about their {@link MonitoredTrip}s.
Expand All @@ -16,8 +17,8 @@ public class TripMonitorNotification extends Model {
private static final Logger LOG = LoggerFactory.getLogger(TripMonitorNotification.class);
public static final String STOPWATCH_ICON = "⏱";

public final NotificationType type;
public final String body;
public NotificationType type;
public String body;

/** Getter functions are used by HTML template renderer */
public String getBody() {
Expand All @@ -28,6 +29,12 @@ public NotificationType getType() {
return type;
}

/**
* This no-arg constructor exists to make MongoDB happy.
*/
public TripMonitorNotification() {
}

public TripMonitorNotification(NotificationType type, String body) {
this.type = type;
this.body = body;
Expand Down Expand Up @@ -110,4 +117,17 @@ public static TripMonitorNotification createInitialReminderNotification(
)
);
}

@Override
public boolean equals(Object o) {
if (o == null || getClass() != o.getClass()) return false;
if (!super.equals(o)) return false;
TripMonitorNotification that = (TripMonitorNotification) o;
return type == that.type && Objects.equals(body, that.body);
}

@Override
public int hashCode() {
return Objects.hash(super.hashCode(), type, body);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ public class JourneyState implements Cloneable {

/**
* The notifications already sent.
* FIXME this is never set, so it has no effect.
*/
public Set<TripMonitorNotification> lastNotifications = new HashSet<>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,6 @@ private static boolean sendAcceptDependentEmail(OtpUser dependentUser, OtpUser r

String acceptDependentLinkLabel = Message.ACCEPT_DEPENDENT_EMAIL_LINK_TEXT.get(locale);
String acceptDependentUrl = getAcceptDependentUrl(acceptKey, locale);
String addressee = (Strings.isBlank(dependentUser.name)) ? dependentUser.email : dependentUser.name;

// A HashMap is needed instead of a Map for template data to be serialized to the template renderer.
Map<String, Object> templateData = new HashMap<>(
Expand All @@ -193,7 +192,7 @@ private static boolean sendAcceptDependentEmail(OtpUser dependentUser, OtpUser r
"acceptDependentLinkLabelAndUrl", label(acceptDependentLinkLabel, acceptDependentUrl, locale),
"acceptDependentUrl", acceptDependentUrl,
"emailFooter", Message.ACCEPT_DEPENDENT_EMAIL_FOOTER.get(locale),
"emailGreeting", String.format(Message.ACCEPT_DEPENDENT_EMAIL_GREETING.get(locale), addressee),
"emailGreeting", String.format(Message.ACCEPT_DEPENDENT_EMAIL_GREETING.get(locale), dependentUser.getDisplayedName()),
"manageLinkUrl", String.format("%s%s", OTP_UI_URL, SETTINGS_PATH),
"manageLinkText", Message.ACCEPT_DEPENDENT_EMAIL_MANAGE.get(locale)
)
Expand Down
Loading
Loading