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 all 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,7 +19,10 @@ public enum Message {
ACCEPT_DEPENDENT_EMAIL_SUBJECT,
ACCEPT_DEPENDENT_EMAIL_MANAGE,
ACCEPT_DEPENDENT_ERROR,
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 @@ -47,6 +50,7 @@ public enum Message {
TRIP_INVITE_COMPANION,
TRIP_INVITE_PRIMARY_TRAVELER,
TRIP_INVITE_OBSERVER,
TRIP_NAME_UNDEFINED,
TRIP_NOT_FOUND_NOTIFICATION,
TRIP_NO_LONGER_POSSIBLE_NOTIFICATION,
TRIP_REMINDER_NOTIFICATION,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
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.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.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;
public TravelerPosition travelerPosition;
public Locale observerLocale;
public TripMonitorNotification tripMonitorNotification;

public LegTransitionNotification(
String travelerName,
NotificationType notificationType,
TravelerPosition travelerPosition,
Locale observerLocale
) {
this.travelerName = travelerName;
this.notificationType = notificationType;
this.travelerPosition = travelerPosition;
this.observerLocale = observerLocale;
this.tripMonitorNotification = createTripMonitorNotification(notificationType);
}

/**
* Create {@link TripMonitorNotification} for leg transition based on notification type.
*/
@Nullable
private TripMonitorNotification createTripMonitorNotification(NotificationType notificationType) {
String body;
switch (notificationType) {
case MODE_CHANGE_NOTIFICATION:
body = String.format(
Message.MODE_CHANGE_NOTIFICATION.get(observerLocale),
travelerName,
travelerPosition.expectedLeg.to.name
);
break;
case DEPARTED_NOTIFICATION:
body = String.format(
Message.DEPARTED_NOTIFICATION.get(observerLocale),
travelerName,
travelerPosition.expectedLeg.from.name
);
break;
case ARRIVED_NOTIFICATION:
body = String.format(
Message.ARRIVED_NOTIFICATION.get(observerLocale),
travelerName,
travelerPosition.expectedLeg.to.name
);
break;
default:
body = null;
}
return (body != null) ? new TripMonitorNotification(notificationType, body) : null;
}

/**
* Get a list of users that should be notified of a traveler's leg transition.
*/
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;
}

/**
* If a traveler is on the route (not deviated), check for possible leg transition notification.
*/
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);
}
}
}
}

/**
* Depending on the traveler's proximity to the start/end of a leg, return the appropriate notification type.
*/
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 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 @@ -510,4 +512,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);
}
}
10 changes: 10 additions & 0 deletions src/main/java/org/opentripplanner/middleware/models/OtpUser.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonSetter;
import org.apache.logging.log4j.util.Strings;
import org.bson.codecs.pojo.annotations.BsonIgnore;
import org.opentripplanner.middleware.auth.Auth0Users;
import org.opentripplanner.middleware.auth.RequestingUser;
import org.opentripplanner.middleware.persistence.Persistence;
Expand Down Expand Up @@ -212,6 +213,15 @@ public Optional<TripSurveyNotification> findLastTripSurveyNotificationSent() {
return tripSurveyNotifications.stream().max(Comparator.comparingLong(n -> n.timeSent.getTime()));
}

/**
* Use name if available, if not fallback on email (which is a required field).
*/
@JsonIgnore
@BsonIgnore
public String getDisplayedName() {
return Strings.isBlank(name) ? email.replace("@", " at ") : name;
}

/** Obtains a notification with the given id, if available. */
public Optional<TripSurveyNotification> findNotification(String id) {
if (tripSurveyNotifications == null || Strings.isBlank(id)) return Optional.empty();
Expand Down
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,22 @@ public static TripMonitorNotification createInitialReminderNotification(
)
);
}
}

/**
* Checks for equality excluding the parent {@link Model} class.
*/
@Override
public boolean equals(Object o) {
if (o == null || getClass() != o.getClass()) return false;
TripMonitorNotification that = (TripMonitorNotification) o;
return type == that.type && Objects.equals(body, that.body);
}

/**
* Creates a hash code from fields in this class only excluding fields within the parent {@link Model} class.
*/
@Override
public int hashCode() {
return Objects.hash(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