Fix loading saved insights

[Twixes]

Changes

Fixes #7439 Fixes #7411. This is similar to #7439 in that the problem here, like there, is setFilters being fired alongside loadInsight in some cases… and in some not – and both can fire loadResults, while we want to avoid fetching results any number of times other than 1 (0 is unacceptable, 2 as well though not that bad). Any suggestions from your side @mariusandra having refactored a lot of this recently?

How did you test this code?

TODO

[mariusandra]

I guess 2 is more than 0. Math definitely checks out. But is there no way to get to 1? This feels like a quick fix without fully untangling the cause.

What route is causing it to load twice? Could adding "if we really didn't do anything by now, just yolo call setFilters here" to the end of urlToAction help?

However, again, 2 > 0, so if this fixes a real issue and doesn't introduce any timing-related edge cases due to the double queries, nor cause too much extra load, I'd be happy to approve. That said, a test is failing. This insight data loading is a very delicate and fragile area, so I tried to have as many cases covered by tests as possible.

PS. You said #7439 twice.

[Twixes]

Oops, here's the correct issue: #7411
This is definitely a quick fix – fixing it 100% will require quite a bit of streamlining to reduce the number of branches that can lead to loadInsight, but I can give that a try too to get to exactly 1 loadInsight call.
Tests are definitely crucial, was just asking if you maybe already had some vision for this logic so I left them TODO.

[mariusandra]

I guess my main insight 🥁 🕶️ ... is that all of this is so flakey, that it's worth investing heavily in tests 😅.

[Twixes]

I spent an inordinate amount of time trying to get to almost exactly 1 results loading always but it's hell to patch every edge case that the current shape of the logic creates. Many actions can directly or indirectly lead to the URL changing, which can lead to those actions… and guards that are in place to prevent these actions from going into a loop are really hard to nail in a way where things load always, but never more times than needed.
Here, the crux of the issue is that all saved insights share one insightLogic instance, and the logic to detect when the currently displayed one has changed (based on the ID) is not 100% reliable. This would definitely be improved with #4329. But in the meantime, I'd prefer to quickly patch this in a suboptimal way (sometimes things may load twice, but that's invisible to the user and not expensive for us – the alternative of things not showing up is worse). I'm happy to refactor the logic more holistically, including things like #4329 and #7376, but in the current shape this feels more like whack-a-mole.