Prevent empty task IDs passed to server side

[ohltyler]

Description

This PR improves the intermediate loading state on the historical results page. Before, an initial load of the page would send requests to the server-side with an empty task ID, which would return a 404 not found due to the path not existing, and show empty results. This is because there would be a double slash // where the task ID was supposed to be in between- e.g., /api/anomaly_detectors/detectors//results/ (note this would only happen if the page was directed via direct URL or refresh, or from the 'View results' button on the detector list page. If navigating from the detector results tab to the historical tab, the issue was not seen since the detector details were already populated in redux store). Typically, this was quickly re-executed with a populated task ID after it was asynchronously fetched, and re-render the page with populated results. Because of this, it was not found as an issue, and was only noticeable when analyzing the network requests from a browser dev tools extension.

To simply prevent these requests from happening at all, we wrap all network requests with an isValidToFetch() fn to prevent empty task IDs from being propagated.

Testing

I've confirmed that results load as normally for HC and non-HC, real-time and historical. Now, the 404 errors are never seen from dev tools since the invalid requests are blocked from occurring in the first place.

Check List

• Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[codecov]

Codecov Report

Merging #616 (69bd976) into main (306f5fa) will increase coverage by 0.02%.
Report is 4 commits behind head on main.
The diff coverage is n/a.

❗ Current head 69bd976 differs from pull request most recent head eaa06c6. Consider uploading reports for the commit eaa06c6 to get more accurate results

@@            Coverage Diff             @@
##             main     #616      +/-   ##
==========================================
+ Coverage   50.28%   50.31%   +0.02%     
==========================================
  Files         166      166              
  Lines        5590     5593       +3     
  Branches     1055     1074      +19     
==========================================
+ Hits         2811     2814       +3     
  Misses       2508     2508              
  Partials      271      271              

see 2 files with indirect coverage changes

[amitgalitz]

if it is not hisotorical even on first load there will be a detector ID?

[ohltyler]

Yes- the AnomalyHistory component is conditionally rendered based on a valid detector, so there will always be a detector ID. (additionally it is required in the url path which cannot be empty). The edge case is that the task ID is asynchronously fetched after the detector ID (via get detector details API) to populate it, which is why the initial render is happening without a task ID.

A different option is to refactor some of the HistoricalDetectorResults page to try to prevent rendering the entire AnomalyHistory component until the task ID is valid. But the current way may provide a better user experience in that it is more react-like, where the only re-rendering is happening in the charts component where the task ID is used to actually populate it once it is fetched and data is retrieved.