Skip to content

Conversation

@dzinad
Copy link
Contributor

@dzinad dzinad commented Apr 10, 2023

No description provided.

@dzinad dzinad requested a review from a team as a code owner April 10, 2023 10:35
@codecov
Copy link

codecov bot commented Apr 10, 2023

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.14%. Comparing base (28ad205) to head (76f0557).
⚠️ Report is 5 commits behind head on main.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1542      +/-   ##
============================================
+ Coverage     78.06%   78.14%   +0.08%     
- Complexity     1062     1070       +8     
============================================
  Files           159      161       +2     
  Lines          4486     4503      +17     
  Branches        626      626              
============================================
+ Hits           3502     3519      +17     
  Misses          715      715              
  Partials        269      269              
Files with missing lines Coverage Δ
.../mapbox/api/directions/v5/models/Notification.java 100.00% <100.00%> (ø)
.../api/directions/v5/models/NotificationDetails.java 100.00% <100.00%> (ø)
.../com/mapbox/api/directions/v5/models/RouteLeg.java 100.00% <ø> (ø)
.../mapbox/api/directions/v5/models/RouteOptions.java 95.40% <100.00%> (+0.02%) ⬆️
...i/directionsrefresh/v1/models/RouteLegRefresh.java 100.00% <ø> (ø)
...com/mapbox/api/directions/v5/MapboxDirections.java 90.07% <100.00%> (+0.15%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@LukasPaczos
Copy link
Contributor

Flagging that backend API design is still in review so we shouldn't merge until that's finalized.

@yuryybk
Copy link

yuryybk commented Aug 12, 2025

@dzinad
Copy link
Contributor Author

dzinad commented Aug 13, 2025

Also, this will be a breaking change for those who used notifications through unrecognized properties.
I believe this is fine since unrecognized properties are supposed to be "beta" aka "unstable and subject to change", but we should issue a warning when we release this and maybe even explicitly tag our customers who we know use it (if there are any).

@RingerJK RingerJK requested a review from Copilot August 13, 2025 09:08

This comment was marked as outdated.

@yuryybk yuryybk requested a review from Copilot August 13, 2025 11:50

This comment was marked as outdated.

@yuryybk yuryybk requested a review from DzmitryFomchyn August 13, 2025 12:20

This comment was marked as outdated.

@yuryybk yuryybk requested a review from Copilot August 13, 2025 13:06

This comment was marked as outdated.

@yuryybk yuryybk requested a review from Copilot August 13, 2025 13:46
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds notification support to the Mapbox Directions API by introducing a new notifications field to RouteLeg and supporting route options. The change enables clients to receive notifications about route violations (like height/weight restrictions) and alerts (like EV charging station availability) within route responses.

Key changes:

  • Added comprehensive notification models (Notification and NotificationDetails) with various notification types and subtypes
  • Added notifications parameter to RouteOptions to control which notifications are included in responses
  • Integrated notifications support across the API service layer and test coverage

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
services-directions-models/src/main/java/com/mapbox/api/directions/v5/models/Notification.java New model class representing route notifications with type, subtype, geometry indices, and details
services-directions-models/src/main/java/com/mapbox/api/directions/v5/models/NotificationDetails.java New model class for notification-specific details like violation values and units
services-directions-models/src/main/java/com/mapbox/api/directions/v5/models/RouteLeg.java Added notifications field to route legs
services-directions-models/src/main/java/com/mapbox/api/directions/v5/models/RouteOptions.java Added notifications parameter to control notification flow
services-directions-models/src/main/java/com/mapbox/api/directions/v5/DirectionsCriteria.java Added extensive constants and annotations for notification types, subtypes, and flow criteria
services-directions/src/main/java/com/mapbox/api/directions/v5/DirectionsService.java Updated API service interface to include notifications parameter
services-directions/src/main/java/com/mapbox/api/directions/v5/MapboxDirections.java Updated API client to pass notifications parameter to service calls

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

@dzinad
Copy link
Contributor Author

dzinad commented Aug 13, 2025

LGTM, but I can't approve my own PR. :)

@yuryybk yuryybk merged commit 454699e into main Aug 13, 2025
4 of 5 checks passed
@yuryybk yuryybk deleted the NAVAND-1268-dd branch August 13, 2025 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants