-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add shmo booking stream event handling #5592
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
base: master
Are you sure you want to change the base?
Conversation
| queryClient.invalidateQueries({ | ||
| queryKey: getActiveShmoBookingQueryKey(acceptLanguage), | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need to pass a language here? Invalidating just "GET_ACTIVE_SHMO_BOOKING_QUERY_KEY" should invalidate queries using both languages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, probably not? I saw that acceptLanguage is required in the query key, and assumed that it couldn't be undefined when used to invalidate either. Pushed a change that makes it optional now, and will test that some.
…and query key functions
ee53e43 to
f77fdd1
Compare
rosvik
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 👍
Dahly96
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look okey, after previous comments are addressed.
One question that might be irrelevant, but are we not receiving the booking data with the event? Could it be set instead of us invalidating and refetching it?
We could, but that would require quite the rewrite, and I think we should think that through in a CODEv or something how we want to implement such handling, so that we have a agreed upon solution for the future. |
|
@Dahly96 Excellent question! Yes, it could be that we should do that. Was thinking to talk about it with the Entur team first to be sure about the content of the events, hopefully in tomorrow's meeting. |
|
Btw that's not a blocker for this PR. In the first round of event handling, it's simplest and safest to start with just ids and invalidation. So if we go for full event data, it should be added later anyway. |
|
@marius-at-atb 100% agree, and like my previous comment we all should be aligned for how we proceed with a shift like that. 😀 |
marius-at-atb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm!
Implement handling for Shmo booking update events in the event stream, allowing the application to invalidate queries related to active Shmo bookings. Instead of setting a
refetchIntervalto poll the update of the price we now invalidate the active booking queries when an event of typeSHMO_BOOKING_UPDATEDappears in the event log.related https://github.com/AtB-AS/kundevendt/issues/21932