Multiple notification channels in middleware #185

JymDyerIBI · 2023-05-17T01:36:07Z

Checklist

Appropriate branch selected (all PRs must first be merged to dev before they can be merged to master)
Any modified or new methods or classes have helpful JavaDoc and code is thoroughly commented
The description lists all applicable issues this PR seeks to resolve
The description lists any configuration setting(s) that differ from the default settings
All tests and CI builds passing
The description lists all relevant PRs included in this release (remove this if not merging to master)

Description

WIP: Middleware preparing to have multiple notification channels, anticipating addition of push notifications.

src/main/java/org/opentripplanner/middleware/models/OtpUser.java

JymDyerIBI · 2023-08-15T18:59:23Z

Requires these configuration properties:

PUSH_API_KEY: STSFYTHREMGUWXML
PUSH_API_URL: https://demo.ibigroupmobile.com/ext_api/otp_push/sound_transit

Note that we only have sound_transit at this time.

binh-dam-ibigroup

Not sure if this PR is supposed to contain fetching of pushDevices, but otherwise LGTM.

src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java

binh-dam-ibigroup

Taking previous approval back... @JymDyerIBI Could you address the test failures, please?

src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java

binh-dam-ibigroup

See https://github.com/ibi-group/otp-middleware/pull/185/files/1fa77f36c8dda4a3f6584f3a4d1e868da5eedc0b..d6c484fe35bc459aa212c90ab0cf9f8ad6683227#r1300616975

binh-dam-ibigroup

One tweak, and with that the PR will be good to go.

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

binh-dam-ibigroup

My apologies, one more change: #185 (comment)

philip-cline

Looks good to me!

src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java

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

binh-dam-ibigroup

As discussed, a check needs to be added to ensure the push notification config params are set before retrieving the number of registered devices and before sending push notifications.

…I_URL. Make inner sendPush() method package scope.

JymDyerIBI · 2023-08-31T21:29:48Z

So this last change will make the public NotificationUtils#sendPush() and #getPushInfo() do nothing and simply return null or 0 respectively if the config properties are not set. There's also an inner sendPush() method that I changed the contract of to not be public so we don't have to duplicate this test.

…I_URL. Make inner sendPush() method package scope.

binh-dam-ibigroup

Thanks for the changes!

Support multiple notification channels.

4321270

JymDyerIBI self-assigned this May 17, 2023

Handle "none" value in notification channel.

0644054

binh-dam-ibigroup mentioned this pull request May 18, 2023

Multiple notification channels opentripplanner/otp-react-redux#899

Merged

3 tasks

binh-dam-ibigroup reviewed May 18, 2023

View reviewed changes

src/main/java/org/opentripplanner/middleware/models/OtpUser.java Show resolved Hide resolved

JymDyerIBI added 8 commits June 8, 2023 12:20

Push notifications.

2715c00

Add pushDevices (number of push devices last we looked)

c3a96c7

Add pushDevices (number of push devices) to test

5f28501

Add sendPush() method for push notifications.

eb2d207

Rely on PUSH_API_URL as config property for entire push API URL.

722143a

Add canSendPushNotification() test.

a109dc6

Generated with VersionControlledSwaggerUpdater

24e825e

Update with PUSH_API_* properties.

1fa77f3

JymDyerIBI assigned binh-dam-ibigroup and unassigned JymDyerIBI Aug 14, 2023

binh-dam-ibigroup approved these changes Aug 15, 2023

View reviewed changes

src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java Outdated Show resolved Hide resolved

binh-dam-ibigroup marked this pull request as ready for review August 15, 2023 20:29

binh-dam-ibigroup assigned JymDyerIBI and unassigned binh-dam-ibigroup Aug 15, 2023

binh-dam-ibigroup requested changes Aug 15, 2023

View reviewed changes

JymDyerIBI added 3 commits August 17, 2023 14:00

Generated with ReadMeEnvSchemaValuesUpdater

13893dd

Space formatting.

3503ee2

Add in get endpoint to maintain number of push notification devices.

d6c484f

binh-dam-ibigroup reviewed Aug 21, 2023

View reviewed changes

src/main/java/org/opentripplanner/middleware/utils/NotificationUtils.java Show resolved Hide resolved

binh-dam-ibigroup requested changes Aug 21, 2023

View reviewed changes

JymDyerIBI added 2 commits August 23, 2023 15:51

Persistence of pushDevices

759d21a

Persistence of pushDevices

ce21e89

binh-dam-ibigroup approved these changes Aug 24, 2023

View reviewed changes

src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java Show resolved Hide resolved

Move NotificationUtils#getPushInfo() call into CheckMonitoredTrip class.

a3e08ab

JymDyerIBI assigned binh-dam-ibigroup and unassigned JymDyerIBI Aug 24, 2023

binh-dam-ibigroup reviewed Aug 24, 2023

View reviewed changes

binh-dam-ibigroup assigned JymDyerIBI and unassigned binh-dam-ibigroup Aug 24, 2023

binh-dam-ibigroup requested a review from philip-cline August 28, 2023 15:48

binh-dam-ibigroup assigned philip-cline and unassigned JymDyerIBI Aug 28, 2023

philip-cline approved these changes Aug 28, 2023

View reviewed changes

src/test/java/org/opentripplanner/middleware/utils/NotificationUtilsTest.java Show resolved Hide resolved

JymDyerIBI added 2 commits August 28, 2023 10:57

Update number of push devices sooner (per binh's PR comment).

fb0df00

Actually, put it very near the very top, after getting otpUser.

be241ed

binh-dam-ibigroup reviewed Aug 28, 2023

View reviewed changes

src/main/java/org/opentripplanner/middleware/tripmonitor/jobs/CheckMonitoredTrip.java Show resolved Hide resolved

binh-dam-ibigroup requested changes Aug 31, 2023

View reviewed changes

binh-dam-ibigroup assigned JymDyerIBI and unassigned philip-cline Aug 31, 2023

Make public static methods contingent on setting PUSH_API_KEY/PUSH_AP…

38660e6

…I_URL. Make inner sendPush() method package scope.

JymDyerIBI assigned binh-dam-ibigroup and unassigned JymDyerIBI Aug 31, 2023

Make public static methods contingent on setting PUSH_API_KEY/PUSH_AP…

17ba7fb

…I_URL. Make inner sendPush() method package scope.

binh-dam-ibigroup approved these changes Sep 1, 2023

View reviewed changes

binh-dam-ibigroup assigned JymDyerIBI and unassigned binh-dam-ibigroup Sep 1, 2023

binh-dam-ibigroup added the BLOCKED Blocked (waiting on another PR to be merged) label Sep 5, 2023

binh-dam-ibigroup merged commit d6ae410 into dev Sep 12, 2023
2 checks passed

binh-dam-ibigroup deleted the multiple-notification-channels branch September 12, 2023 12:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multiple notification channels in middleware #185

Multiple notification channels in middleware #185

JymDyerIBI commented May 17, 2023 •

edited

Loading

JymDyerIBI commented Aug 15, 2023

binh-dam-ibigroup left a comment

binh-dam-ibigroup left a comment

binh-dam-ibigroup left a comment

binh-dam-ibigroup left a comment

binh-dam-ibigroup left a comment

philip-cline left a comment

binh-dam-ibigroup left a comment

JymDyerIBI commented Aug 31, 2023 •

edited

Loading

binh-dam-ibigroup left a comment

Multiple notification channels in middleware #185

Multiple notification channels in middleware #185

Conversation

JymDyerIBI commented May 17, 2023 • edited Loading

Checklist

Description

JymDyerIBI commented Aug 15, 2023

binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

philip-cline left a comment

Choose a reason for hiding this comment

binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

JymDyerIBI commented Aug 31, 2023 • edited Loading

binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

JymDyerIBI commented May 17, 2023 •

edited

Loading

JymDyerIBI commented Aug 31, 2023 •

edited

Loading