Skip to content

feat: Add conditional min_transfer_time validation - #2175

Open
mackenziereading19 wants to merge 2 commits into
MobilityData:masterfrom
mackenziereading19:2174-conditional-min-transfer-time
Open

feat: Add conditional min_transfer_time validation#2175
mackenziereading19 wants to merge 2 commits into
MobilityData:masterfrom
mackenziereading19:2174-conditional-min-transfer-time

Conversation

@mackenziereading19

Copy link
Copy Markdown

Summary:

Implements #2174 by adding validation for the conditionally required transfers.min_transfer_time field when transfer_type = 2 (MINIMUM_TIME).

Expected behavior:

When transfer_type = 2 and min_transfer_time is missing or empty, the validator emits the existing missing_required_field ERROR notice for min_transfer_time in transfers.txt.

A supplied value of 0 remains valid. Other transfer types do not require min_transfer_time.

Implementation:

  • Mark min_transfer_time as @ConditionallyRequired in GtfsTransferSchema.
  • Add TransferMinTransferTimeConditionalValidator.
  • Reuse the existing MissingRequiredFieldNotice.
  • Add focused unit tests covering missing, zero, positive, and non-MINIMUM_TIME cases.

Validation:

  • Focused validator tests: 4/4 passed.
  • Existing transfer validation tests: 3/3 passed.
  • End-to-end CLI validation reproduced the expected missing_required_field notice.
  • Spotless formatting check passed.
  • Full :main:test was run. Three unrelated Mockito-based tests also fail on the clean parent commit, confirming they are pre-existing and not introduced by this change.
  • No documentation changes are required for this focused validator.
  • Linked issue: Add notice for conditionally required min_transfer_time #2174.
  • Screenshots: not applicable for this validator change.

Closes #2174

@welcome

welcome Bot commented Aug 20, 2026

Copy link
Copy Markdown

Thanks for opening this pull request! You're awesome. We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix. Examples of titles with semantic prefixes:

  • fix: Bug with ssl network connections + Java module permissions.
  • feat: Initial support for multiple @PrimaryKey annotations.
  • docs: update RELEASE.md with new process
    To get this PR to the finish line, please do the following:
  • Read our Contribution Guidelines
  • Follow Google Java style coding standards
  • Include tests when adding/changing behavior
  • Include screenshots

@CLAassistant

CLAassistant commented Aug 20, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@mackenziereading19 mackenziereading19 changed the title Add conditional min_transfer_time validation feat: Add conditional min_transfer_time validation Aug 20, 2026
@emmambd
emmambd requested a review from skalexch August 24, 2026 14:06

@skalexch skalexch left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good logic and test-wise.

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.

Add notice for conditionally required min_transfer_time

5 participants