Fix initialization callouts to properly show when first loading anomaly results page #300
Conversation
ohltyler
force-pushed
the
fixInitCallout
branch
from
b6ee4bf to
48be6c7
Compare
| await dispatch(getDetector(detectorId)); | ||
| dispatch(getDetector(detectorId)); |
There was a problem hiding this comment.
wondering why removing await here?
There was a problem hiding this comment.
Unnecessary, since dispatch() is just used to trigger a separate async action to make the api call and update the redux store, and the fn itself is already async. Also, nothing is done after the action is done within this function (like catch errors, do something with the response, etc) if we were to await. VS code also complains about it being unnecessary / has no effect.
Once the action is done, the page will render with the updated detector since the redux store will be updated.
There was a problem hiding this comment.
VS code also complains about it being unnecessary / has no effect.
Hmm, why I didn't see that in my VS code. is it because of any good plugin?
There was a problem hiding this comment.
Not sure, don't have extensions specific to javascript/typescript/react. Just looks like this. Have also noticed it in many other places, just fixed here though.
Issue #, if available: #299
Description of changes:
This PR fixes the issue of the anomaly results page not properly showing the callouts relating to init progress (if applicable) when first loading the page after detector creation. Specifically, this fix will re-get the detector details when first rendering the page, to update the redux store with the detector details if they haven't been updated already.
The issue (before): user creates detector -> redux store gets updated with detector info returned by create detector api (which doesn't include any info related to init progress) -> user clicks on anomaly results page -> page uses existing detector info -> assumes no init progress -> renders old callout. Note that this issue only comes up if the store hasn't already been updated by rendering some other page, like detector list or dashboard for example, where getDetectors() is called, which would update the redux store with up-to-date detector details.
Confirmed all UT/IT pass.
Will cherry-pick to opendistro-1.10 branch and push new tag.
Screenshots:
Before:
After:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.