-
Notifications
You must be signed in to change notification settings - Fork 33
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
Resolves #3220 - show heartbeat status in top bar #3252
Conversation
… tests for sessionInfoBar still.
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.
Thanks for updating this to use Redux + Sagas. I spun up this issue for us to track the conversation for adopting new state management / data fetching patterns.
For this PR:
- I see you elected to add a new
heartbeat
slice of the store. Any reason you didn't opt too add a property in thesession
slice, e.g.session.heartbeat
? We are adding this to theSessionInfoBar
, after all - Adding a few saga tests (see these examples for session sagas) might be a good idea
Mainly because the session slice is written to |
Added a few tests for the saga. |
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 you think that's a reasonable place for it though?
It does feel more reasonable to have this in the session
slice, but yeah, doesn't make sense to save to local storage. Maybe we could update saveSession
/ loadSession
to account for that.
However, seeing as this is blocking a Remote Settings release. Maybe we can call this PR good for now and create a follow on issue.
I think all we need to call this PR done is 1 test that demonstrates that the heartbeat is re-fetched after the timeout winds down.
Resolves #3220 . Show the heartbeat endpoint status in top bar. Status icon is also a link that will take the user to the
__heartbeat__
endpoint. Also resolves a brief error that users see during redirect to auth.Screenshots of error (red !) vs success (green circle):