Skip to content

feat: Remove service-day - #1752

Merged
arielvino merged 31 commits into
mainfrom
refactor/remove-service-day
Aug 18, 2026
Merged

feat: Remove service-day#1752
arielvino merged 31 commits into
mainfrom
refactor/remove-service-day

Conversation

@arielvino

@arielvino arielvino commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

Description

  • Remove service-day logic and its usage from pages: gaps, singleLineMap, lineProfile, vehicle.
  • Add a notice at the bottom of GapsTable that warn about next-date rides.

Why?

The real reason

It adds the need for per-hour granularity for data that we only hold in date granularity.
For example:

  • ServiceDayRoutesService was implemented to derive routes in specific hours - as they only exists in the source/backend as per-date.
  • In feat: Make line-selector an autocomplete #1701 its needed to invent per-hour derivation for operator and for line numbers.

The UX trade-off

Pros

  • Late night hours are intuitively perceived as belong to the previous date - so the user can find them there.

Cons

  • Night rides appear twice - in their real date and in the date before them. This can sometimes be confusing.
  • It require non-intuitive workarounds for persisting or transferring data between pages. For example the 25:00+ clock format.

Notes

  • The 25:00+ clock format does not exist in the database. It sometimes exists in the source, but the ETL normalize it to the exact date. So the feature is purely frontend.
  • The API has a bug that wire startTimeTo to the end_time of the ride instead of the start_time. A fix submitted in fix: start_time_to was bound to end_time instead of start_time open-bus-stride-api#59, but till it merged - rides that start within the time window but end after it will be excluded. (pre-existing, but now it will affect more day-time rides).
  • The original issue נסיעות לאחר חצות #1333 bug already got sufficient fix by pulling vehicle-locations by siri rides - so it doesn't depend on the service-day window.

screenshots

image image

@github-actions

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

@arielvino
arielvino marked this pull request as ready for review July 28, 2026 15:11
@arielvino
arielvino requested a review from AvivAbachi as a code owner July 28, 2026 15:11
@arielvino
arielvino requested a review from NoamGaash July 28, 2026 15:11
@NoamGaash

Copy link
Copy Markdown
Member

maybe we should ask the backend to add fields for service day and service hour?

@arielvino

Copy link
Copy Markdown
Collaborator Author

@NoamGaash
Then it will move the same complexity to the backend.
Because at the source siri/gtfs this data exists only per date - and the ETL will need to calculate it.

Also the complexity of transferring data between pages (gaps / singleLineMap / vehicle) is inherent to the frontend.

@NoamGaash NoamGaash left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry for taking that long to review it - I don't have the time capacity right now and that's confusing me.
I have a feeling that our ETL pipelines should preserve the information about the service day, the time, and the real timestamp, so the frontend will be able to decide which piece of information should be used for each context, but when I asked Claude for help explaining it he took your side - I am sure I'm missing something here and I'll spend some time making this research later, but for now I'm under the impression it's a good healthy change (and also I have a lot of trust in your research).
Sorry for taking so long - approved - and thank you for your dedication here 💪

@sonarqubecloud

Copy link
Copy Markdown

@arielvino
arielvino merged commit 4be4048 into main Aug 18, 2026
22 checks passed
@arielvino
arielvino deleted the refactor/remove-service-day branch August 18, 2026 17:13
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.

3 participants