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

Realtime stops vectortile layer checks whether there are stoptimes on the service day #5861

Merged

Conversation

sharhio
Copy link
Contributor

@sharhio sharhio commented May 22, 2024

PR Instructions

Summary

A badge needs to be displayed on the map for stops without traffic on the service day. The vectortile layer for realtime stops checks if stop times exist for a particular stop and returns the result.

Issue

  • A badge needs to be displayed on the map for stops without traffic on the service day.

Unit tests

  • Added the new value to the RealtimeStopsLayerTest.
  • Functionality was manually verified.

List.of(new KeyValue("closedByServiceAlert", noServiceAlert))
List.of(
new KeyValue("closedByServiceAlert", noServiceAlert),
new KeyValue("noServiceOnServiceDay", !stopTimesExist)
Copy link
Member

Choose a reason for hiding this comment

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

Would it be possible to turn this around? Basically you state what exists (services) and not what doesn't exist?

Copy link
Member

Choose a reason for hiding this comment

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

Double negatives (noService=false) always make my head hurt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course! Sorry about that.

@sharhio sharhio requested a review from leonardehrenfried May 23, 2024 11:46
Copy link

codecov bot commented May 23, 2024

Codecov Report

Attention: Patch coverage is 87.50000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 68.38%. Comparing base (fe41603) to head (ae3a95a).
Report is 41 commits behind head on dev-2.x.

Files Patch % Lines
...s/stops/DigitransitRealtimeStopPropertyMapper.java 87.50% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #5861      +/-   ##
=============================================
+ Coverage      68.37%   68.38%   +0.01%     
- Complexity     16673    16678       +5     
=============================================
  Files           1914     1914              
  Lines          72652    72659       +7     
  Branches        7452     7452              
=============================================
+ Hits           49673    49686      +13     
+ Misses         20422    20416       -6     
  Partials        2557     2557              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…DigitransitRealtimeStopPropertyMapper.java

Co-authored-by: Leonard Ehrenfried <[email protected]>
List.of(new KeyValue("closedByServiceAlert", noServiceAlert))
List.of(
new KeyValue("closedByServiceAlert", noServiceAlert),
new KeyValue("servicesRunningOnServiceDay", stopTimesExist)
Copy link
Member

Choose a reason for hiding this comment

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

I think most of the time we call it ServiceDate instead of ServiceDay.

@@ -32,10 +34,19 @@ protected Collection<KeyValue> map(RegularStop stop) {
.stream()
.anyMatch(alert -> alert.noServiceAt(currentTime));

var serviceDate = LocalDate.now(transitService.getTimeZone());
boolean stopTimesExist = transitService
.getStopTimesForStop(stop, serviceDate, ArrivalDeparture.BOTH, true)
Copy link
Member

Choose a reason for hiding this comment

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

I tried to quickly take a look if this also includes stop times for patterns that have been added through an realtime update but I'm not sure. However, if it doesn't, I think we have plenty of other api calls which are also broken in that sense so maybe we shouldn't care about it for now.

Copy link
Contributor Author

@sharhio sharhio May 27, 2024

Choose a reason for hiding this comment

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

It seems like the realtimes are added as the stopTimesHelper.stopTimesForStop fetches the patterns with includeRealtimeUpdates set as true.

@vesameskanen vesameskanen merged commit 82c0c58 into opentripplanner:dev-2.x May 28, 2024
5 checks passed
@vesameskanen vesameskanen deleted the realtimestop-layer-no-stoptimes branch May 28, 2024 09:22
@t2gran t2gran added this to the 2.6 (next release) milestone Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants