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

Change of dates make line data disappear #538

Closed
yael-naor opened this issue Feb 26, 2024 · 9 comments · Fixed by #741
Closed

Change of dates make line data disappear #538

yael-naor opened this issue Feb 26, 2024 · 9 comments · Fixed by #741
Assignees
Labels
frontend frontend developers issue good first issue Good for newcomers

Comments

@yael-naor
Copy link

On the page "אחוזי יציאה מכלל הנסיעות":
I'm requesting information on route 24 from Metropolin company.
When I request dates 5/2/24 until 25/2/24, I receive valid information
However, when I request dates 4/2/24 until 25/2/24, I receive a message that there's not such line.
F12 shows that the data exists for the latter dates as well.

@NoamGaash NoamGaash added good first issue Good for newcomers frontend frontend developers issue labels Feb 26, 2024
@InbarSle
Copy link

Can i be assigned to this?

@shootermv
Copy link
Collaborator

Of course you can!

@YuvalMasada
Copy link
Collaborator

YuvalMasada commented Mar 21, 2024

Hi @yael-naor ,
Can you reproduce this issue every time you play with the dates?
I tried to reproduce and I couldn't. I am not sure if I did the correct process to reproduce it.

@itsoriki
Copy link
Collaborator

itsoriki commented May 5, 2024

@NoamGaash I can't reproduce the issue - it might be irrelevant now

@NoamGaash
Copy link
Member

@itsoriki did you try the right page and settings?
image
image

@itsoriki
Copy link
Collaborator

itsoriki commented May 6, 2024

Now I see it :)

@itsoriki itsoriki self-assigned this May 7, 2024
@itsoriki
Copy link
Collaborator

itsoriki commented May 8, 2024

@NoamGaash After a serious investigation I have come up with the following:
The problem is in the gtfsService
There are some issues I found there:

  • In line 32 there is a filter of the gtfsRoutes with an interesting condition (see picture)
    image
    This filter removes all the routes except for the ones with the date of the to parameter - meaning the routes in the last day of the search range (In our example the search range is from 5/2/24 to 25/2/24, so the filter will result only the routes of the 25th). Now, when we extend the search range to be from the 4/2/24, there are more than 100 routes the gtfs finds, but the limit is only 100 (see picture)
    image
    So we end up getting just the first 100 with the routes of the final day (25th) being left out of the array - and then when the filter does its thing - all the routes get filtered out and the result in the UI is "הקו לא נמצא".
    Btw, even when we search from 5/2/24 and the UI shows some results - they are not the "whole story" and some routes are left out (2 to be precise) because of the limit: 100.
    Now, I saw that this filter is meant to solve another problem mentioned in fix: Use Only Relevant Routes #244 - so we need to figure out a better way to do things here.
  • The limit of the gtfsRoutes query is 100 but there are more than 100 in our case for example - and the ones that left out (the ones of the 25/2/24) are apparently the important ones (I assume they are important because we filter for them and only them in the code as seen above)
  • The GTFS API returns also the routes of the day before the from day (In our example when we search for 5/2/24 - 25/2/24, the api gives us also the routes of the 4/2/24 - which is the one day over what we asked for. I saw that @ArkadiK94 referred to this in his fix: Use Only Relevant Routes #244 PR as "the routes of yesterday problem" - so I guess you are aware of the issue

Conclusion

We parse the routes from the GTFS API in a way that might and actually leads to problems - Let's discuss also changes in the backend to address the problem as a whole!

@NoamGaash
Copy link
Member

@ShayAdler You got to see this 🤯
@itsoriki Thank you! That's an amazing investigation 👏
seems like this method assumes that the fromTimestamp and toTimestamp belong to the same date, which is obviously wrong in our case.
maybe we should change to:

    .filter((route) => route.date.getDate() <= toTimestamp.date() && route.date.getDate() >= fromTimestamp.date())

or something similar (?)

@itsoriki
Copy link
Collaborator

itsoriki commented May 8, 2024

That's exactly what I had in mind! I'll fix it right away :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend frontend developers issue good first issue Good for newcomers
Projects
None yet
6 participants