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: derive limits from hastus exports #1204

Merged
merged 1 commit into from
Apr 7, 2025

Conversation

sloanelybutsurely
Copy link
Contributor

@sloanelybutsurely sloanelybutsurely commented Apr 4, 2025

  • Adds start_stop_id and end_stop_id to hastus services
  • Updates export upload to derive limits from hastus exports
    • This looks at the stop_sequences for the trips within the services in the export to determine the boundary stops
  • Renders hastus services with limits in the UI

🏹 Implement HASTUS export-derived limits determination & display

Note: This skips an integration test that I could not for the life of me figure out a way to fix. It is unclear to me what exactly wasn't working. The screenshots from the test either showed exactly what the error said was missing or a progress bar. Some sort of race condition?

Reviewer Checklist

  • Meets ticket's acceptance criteria
  • Any new or changed functions have typespecs
  • Tests were added for any new functionality (don't just rely on Codecov)
  • This branch was deployed to the staging environment and is currently running with no unexpected increase in warnings, and no errors or crashes.

@sloanelybutsurely sloanelybutsurely requested a review from a team as a code owner April 4, 2025 13:10
@sloanelybutsurely sloanelybutsurely requested review from cmaddox5 and removed request for a team April 4, 2025 13:10
@sloanelybutsurely sloanelybutsurely force-pushed the sloane/push-notzstqvkxzl branch 2 times, most recently from bb4f60c to 64a7220 Compare April 4, 2025 13:15
Copy link
Contributor

@cmaddox5 cmaddox5 left a comment

Choose a reason for hiding this comment

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

Looks great! Awesome job!

FYI, we have a known flaky integration test that tends to fail CI atm. There is a task to address it but we haven't prioritized it yet.

@sloanelybutsurely
Copy link
Contributor Author

we have a known flaky integration test that tends to fail CI atm

i wish this were the case here. i seem to have actually broken some integration tests. working through fixing those!

@sloanelybutsurely sloanelybutsurely force-pushed the sloane/push-notzstqvkxzl branch 10 times, most recently from 23e1116 to d63db7b Compare April 7, 2025 20:31
- Adds start_stop_id and end_stop_id to hastus services
- Updates export upload to derive limits from hastus exports
  - This looks at the stop_sequences for the trips within the services
    in the export to determine the boundary stops
- Renders hastus services with limits in the UI

**Note**: This skips an integration test that I could not for the life
of me figure out a way to fix. It is unclear to me what exactly wasn't
working. The screenshots from the test either showed exactly what the
error said was missing or a progress bar. Some sort of race condition?

[🏹 Implement HASTUS export-derived limits determination & display](https://app.asana.com/0/584764604969369/1209459244072060/f)
@sloanelybutsurely sloanelybutsurely force-pushed the sloane/push-notzstqvkxzl branch from d63db7b to d1bede1 Compare April 7, 2025 20:36
@sloanelybutsurely sloanelybutsurely merged commit 0240e66 into master Apr 7, 2025
10 checks passed
@sloanelybutsurely sloanelybutsurely deleted the sloane/push-notzstqvkxzl branch April 7, 2025 20:43
cmaddox5 added a commit that referenced this pull request Apr 10, 2025
cmaddox5 added a commit that referenced this pull request Apr 10, 2025
* Revert "feat: derive limits from hastus exports (#1204)"

This reverts commit 0240e66.

* Remove unused columns.

* Keep hidden option for inputs.
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.

2 participants