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.

Sign up for GitHub

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

feat: modified MissingPickupDropOffBookingRuleIdNotice logic #2001

Merged
merged 6 commits into from
Mar 4, 2025
+12 −3
Merged

feat: modified MissingPickupDropOffBookingRuleIdNotice logic #2001

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fdd7413
modified MissingPickupDropOffBookingRuleIdNotice logic
qcdyx Mar 4, 2025
0745fe0
reverted test name
qcdyx Mar 4, 2025
e9883a1
Merge branch 'master' into 1821-booking_rule
qcdyx Mar 4, 2025
d291796
Merge branch 'master' into 1821-booking_rule
qcdyx Mar 4, 2025
9ae0e9c
Merge branch 'master' into 1821-booking_rule
qcdyx Mar 4, 2025
0626d40
Merge branch 'master' into 1821-booking_rule
qcdyx Mar 4, 2025
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
10 changes: 8 additions & 2 deletions .../src/main/java/org/mobilitydata/gtfsvalidator/validator/PickupBookingRuleIdValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public PickupBookingRuleIdValidator(
public void validate(GtfsStopTime entity, NoticeContainer noticeContainer) {
if (entity.hasPickupType()
&& entity.pickupType() == GtfsPickupDropOff.MUST_PHONE
&& entity.hasStartPickupDropOffWindow()
Copy link
Contributor

Choose a reason for hiding this comment

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

I might misunderstand this comment: #1821 (comment)
But shouldn't it also check for entity.hasEndPickupDropOffWindow() here?

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 don't think so based on the revised logic:

  1. GtfsStopTime has a pickupType of 2, includes StartPickupDropOffWindow AND has a missing pickupBookingRuleId.
  2. GtfsStopTime has a dropOffType of 2 and includes EndPickupDropOffWindow AND has a missing dropOffBookingRuleId.
    If one of the conditions is met, it should trigger MissingPickupDropOffBookingRuleIdNotice.

&& !entity.hasPickupBookingRuleId()) {
noticeContainer.addValidationNotice(
new MissingPickupDropOffBookingRuleIdNotice(
Expand All @@ -34,6 +35,7 @@ public void validate(GtfsStopTime entity, NoticeContainer noticeContainer) {
}
if (entity.hasDropOffType()
&& entity.dropOffType() == GtfsPickupDropOff.MUST_PHONE
&& entity.hasEndPickupDropOffWindow()
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

&& !entity.hasDropOffBookingRuleId()) {
noticeContainer.addValidationNotice(
new MissingPickupDropOffBookingRuleIdNotice(
Expand Down Expand Up @@ -61,8 +63,12 @@ public boolean shouldCallValidate() {
}

/**
* `pickup_booking_rule_id` is recommended when `pickup_type=2` and `drop_off_booking_rule_id` is
* recommended when `drop_off_type=2`
* pickup_booking_rule_id is recommended when pickup_type=2 and drop_off_booking_rule_id is
* recommended when drop_off_type=2.
*
* <p>Currently, this notice is only triggered on feeds when either start_pickup_drop_off_window
* or end_pickup_drop_off_window is defined, since this recommendation was added to the
* specification for feeds with GTFS-Flex.
*/
@GtfsValidationNotice(
severity = SeverityLevel.WARNING,
Expand Down
5 changes: 4 additions & 1 deletion .../test/java/org/mobilitydata/gtfsvalidator/validator/PickupBookingRuleIdValidatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.mobilitydata.gtfsvalidator.notice.ValidationNotice;
import org.mobilitydata.gtfsvalidator.table.GtfsPickupDropOff;
import org.mobilitydata.gtfsvalidator.table.GtfsStopTime;
import org.mobilitydata.gtfsvalidator.type.GtfsTime;

@RunWith(JUnit4.class)
public class PickupBookingRuleIdValidatorTest {
Expand All @@ -27,6 +28,7 @@ public void missingBookingRuleIdShouldGenerateNotice() {
new GtfsStopTime.Builder()
.setCsvRowNumber(1)
.setPickupType(GtfsPickupDropOff.MUST_PHONE)
.setStartPickupDropOffWindow(GtfsTime.fromSecondsSinceMidnight(18614))
.build();
assertThat(generateNotices(stopTime))
.containsExactly(
Expand All @@ -40,7 +42,8 @@ public void existingBookingRuleIdShouldNotGenerateNotice() {
new GtfsStopTime.Builder()
.setCsvRowNumber(2)
.setPickupType(GtfsPickupDropOff.MUST_PHONE)
.setPickupBookingRuleId("bookingRuleId")
.setStartPickupDropOffWindow(GtfsTime.fromSecondsSinceMidnight(18614))
.setPickupBookingRuleId("booking_rule_id")
.build();
assertThat(generateNotices(stopTime)).isEmpty();
}
Expand Down
Loading