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

feat: added forbidden_prior_day_booking_field_value, invalid_prior_notice_duration_min and forbidden_prior_notice_start_day notices #1860

Merged
merged 4 commits into from
Oct 2, 2024

Conversation

cka-y
Copy link
Contributor

@cka-y cka-y commented Oct 1, 2024

Summary:

This PR introduces three new validation notices: forbidden_prior_day_booking_field_value, invalid_prior_notice_duration_min, and forbidden_prior_notice_start_day.

  • forbidden_prior_day_booking_field_value:

    • Error Logic:
      • Trigger when booking_type = 2 and either prior_notice_duration_min or prior_notice_duration_max is present.
  • invalid_prior_notice_duration_min:

    • Error Logic:
      • Trigger when prior_notice_duration_min is greater than prior_notice_duration_max.
  • forbidden_prior_notice_start_day:

    • Error Logic:
      • Trigger when prior_notice_start_day is defined and prior_notice_duration_max is also defined.

Expected Behavior:

A validation notice with ERROR severity is generated when any of the above conditions are met for each specific notice.

Using this modified feed, the following validation results were obtained:
Screenshot 2024-10-02 at 11 18 38 AM

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Run the unit tests with gradle test to make sure you didn't break anything
  • Add or update any needed documentation to the repo
  • Format the title like "feat: [new feature short description]". Title must follow the Conventional Commit Specification(https://www.conventionalcommits.org/en/v1.0.0/).
  • Linked all relevant issues
  • Include screenshot(s) showing how this pull request works and fixes the issue(s)

cka-y added 2 commits October 1, 2024 12:56
…e_duration_min` and `forbidden_prior_notice_start_day` notices
Copy link
Contributor

github-actions bot commented Oct 1, 2024

📝 Acceptance Test Report

📋 Summary

✅ The rule acceptance has passed for commit 208afa4
Download the full acceptance test report here (report will disappear after 90 days).

📊 Notices Comparison

New Errors (0 out of 1588 datasets, ~0%) ✅

No changes were detected due to the code change.

Dropped Errors (0 out of 1588 datasets, ~0%) ✅

No changes were detected due to the code change.

New Warnings (0 out of 1588 datasets, ~0%) ✅

No changes were detected due to the code change.

Dropped Warnings (0 out of 1588 datasets, ~0%) ✅

No changes were detected due to the code change.

🛡️ Corruption Check

0 out of 1588 sources (~0 %) are corrupted.

⏱️ Performance Assessment

📈 Validation Time

Assess the performance in terms of seconds taken for the validation process.

Time Metric Dataset ID Reference (s) Latest (s) Difference (s)
Average -- 4.06 4.11 ⬆️+0.05
Median -- 1.40 1.45 ⬆️+0.05
Standard Deviation -- 11.61 11.61 ⬇️-0.00
Minimum in References Reports us-california-city-of-wasco-gtfs-1788 0.50 0.53 ⬆️+0.03
Maximum in Reference Reports gb-unknown-uk-aggregate-feed-gtfs-2014 292.43 291.18 ⬇️-1.25
Minimum in Latest Reports us-oregon-hut-airport-shuttle-gtfs-635 0.54 0.52 ⬇️-0.02
Maximum in Latest Reports gb-unknown-uk-aggregate-feed-gtfs-2014 292.43 291.18 ⬇️-1.25

@emmambd
Copy link
Contributor

emmambd commented Oct 1, 2024

@cka-y On a QA side, works as expected!

@cka-y cka-y requested a review from davidgamez October 1, 2024 19:00
@emmambd
Copy link
Contributor

emmambd commented Oct 1, 2024

@cka-y For forbidden_prior_notice_start_day, I just realized the "invalid value" language might be confusing, since the field itself is forbidden in cases where prior_notice_duration_max is provided, not just that there's an invalid specific inputted value for it.

Suggested change: "prior_notice_start_day is forbidden when prior_notice_duration_max is set"

Copy link
Contributor

@qcdyx qcdyx left a comment

Choose a reason for hiding this comment

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

LGTM

@cka-y cka-y force-pushed the feat/booking-rules-validations branch from 4a2cdf0 to 49baccf Compare October 2, 2024 14:58
@cka-y
Copy link
Contributor Author

cka-y commented Oct 2, 2024

done @emmambd -- waiting for @tzujenchanmbd approval before merging

@tzujenchanmbd
Copy link

LGTM on columns displayed, notice names, and description.

Copy link
Contributor

github-actions bot commented Oct 2, 2024

📝 Acceptance Test Report

📋 Summary

✅ The rule acceptance has passed for commit 566fa52
Download the full acceptance test report here (report will disappear after 90 days).

📊 Notices Comparison

New Errors (0 out of 1588 datasets, ~0%) ✅

No changes were detected due to the code change.

Dropped Errors (0 out of 1588 datasets, ~0%) ✅

No changes were detected due to the code change.

New Warnings (0 out of 1588 datasets, ~0%) ✅

No changes were detected due to the code change.

Dropped Warnings (0 out of 1588 datasets, ~0%) ✅

No changes were detected due to the code change.

🛡️ Corruption Check

0 out of 1588 sources (~0 %) are corrupted.

⏱️ Performance Assessment

📈 Validation Time

Assess the performance in terms of seconds taken for the validation process.

Time Metric Dataset ID Reference (s) Latest (s) Difference (s)
Average -- 4.10 4.19 ⬆️+0.08
Median -- 1.46 1.51 ⬆️+0.05
Standard Deviation -- 11.63 11.60 ⬇️-0.02
Minimum in References Reports au-tasmania-merseylink-gtfs-1251 0.52 0.60 ⬆️+0.08
Maximum in Reference Reports gb-unknown-uk-aggregate-feed-gtfs-2014 294.81 291.40 ⬇️-3.41
Minimum in Latest Reports ar-buenos-aires-subterraneos-de-buenos-aires-subte-gtfs-6 0.58 0.54 ⬇️-0.04
Maximum in Latest Reports gb-unknown-uk-aggregate-feed-gtfs-2014 294.81 291.40 ⬇️-3.41

@cka-y cka-y merged commit 817c6e4 into master Oct 2, 2024
335 checks passed
@cka-y cka-y deleted the feat/booking-rules-validations branch October 2, 2024 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants