AtB-AS / atb-bff

BFF for mittatb-web and mittatb-app
European Union Public License 1.2
2 stars 0 forks source link

bug: display missing departures in favourites #342

Open jonasbrunvoll opened 3 months ago

jonasbrunvoll commented 3 months ago

Closes https://github.com/AtB-AS/kundevendt/issues/15921

Background

From FRAM: Skrenten was set up as the favorite departure with 701 All line variations. It doesn't show all departures? Missing here at 16:24, 16:54, 17:24. An actual travel search and the menu item Departures show all departures.

Illustrations

screenshots/video/figma | Image 1 | Image 2 | | -------- | ------- | | ![Simulator Screenshot - iPhone SE (3rd generation) - 2024-07-31 at 15 28 02](https://github.com/user-attachments/assets/95035b08-d2a2-4418-9303-5d3ee445b33f) | ![Simulator Screenshot - iPhone SE (3rd generation) - 2024-07-31 at 15 29 41](https://github.com/user-attachments/assets/7da65f28-905a-4f2b-9a16-78f8fe52b7e6) |

Proposed solution

Removing the property numberOfDeparturesPerLineAndDestinationDisplay, as it serves none useful purpose in this query.

Acceptance criteria

Test input

mikaelbr commented 3 months ago

We are talking 4 years back, written for a different view so I must admit I don't remember all the details 🤕 But what you say was the motivation for using it (to get actual unique estimated calls). But to understand the consequences here, in the context of the favourite widget on the front page, I would have to investigate some and refresh my memory.

gorandalum commented 3 months ago

I'll do a quick local test, to see if there are any glaring issues.

gorandalum commented 3 months ago

By testing and looking at the code technically it works in most situations because we use lodash.groupBy and use the first item in the returned collection. So even though we now end up grouping multiple estimated calls by the same key, we end up using just one of them.

One issue would be that the estimatedCalls part of the query now lists every departure, so it can be filled by departures for the most frequent line, and this not reach the more infrequent lines. We send in a total limit of 70, so it wouldn't happen often, but it will probably happen in some cases.

Another issue regarding the case of Skrenten is that numberOfDeparturesPerLineAndDestinationDisplay is still used in the times part of the query. This means that the functionality to get 7 departures per line name still won't work fully as intended.

(We can do a Huddle if these issues doesn't make sense, it's hard to put them down to words)

All in all there are some minor setbacks with this change, and I'm still not fully aware if I have touched all of them. It's hard for me to say if the benefit it worth the risk or not. I would advice that the main focus is on getting the fix in OTP in the september release, through pointing out that it is important to us and that we as of now have a bug in production.

jonasbrunvoll commented 3 months ago

In the tread https://mittatb.slack.com/archives/CHLDG8C30/p1701941499325799, Entur created an issue to update the query stopPlaces making it return multiple destinationDisplays when dissimilar values in via. The issue is planed to be included in the 2.6 release, 15. september. https://mittatb.slack.com/archives/CHLDG8C30/p1721811197387069. I will pause working on this task until after publication, in case the issue will be resolved by Entur.