So I was working through the process of enabling monitoring while having some misconfigurations in my cluster settings. For example, I had monitoring disabled, but I also had a local exporter configured that was also disabled (a local exporter disabled will show an error on this page). When I clicked the blue button to enable monitoring, the spinner never left, but after some debugging I realized it was because this.isCollectionEnabledUpdated was set to true and never reset.

I don't know if the right change is to remove this line from the conditional, or maybe look into how it's mutated to understand if a change is necessary there, but I think we can do a little more to make a more seamless UX.

Looks like it's only set in the catch but never reseted:

I will set it to false before the async function here:

I don't think this solution quite works. I'm not seeing a change in behavior on my side. It's still stuck on true which is preventing the next startChecks() call

Maybe we're testing different scenarios. What steps are taking to reproduce the sticky loading phase?

Sure, so start off with a cluster that has no monitoring data and does not have monitoring enabled. Go to the dev tools and configure a local or remote exporter that is disabled, something like:

PUT _cluster/settings
{
  "transient": {
    "xpack.monitoring.exporters": { 
      "my_remote": {
        "type": "http",
        "enabled": false
      }
    }
  }
}

Go to the monitoring UI, see the blue enable button, and click it. My experience is it just keeps spinning and never shows me the next error (which is that all my exporters are disabled).

This is where I found the list of scenarios that would cause an error message: https://github.com/elastic/kibana/blob/master/x-pack/legacy/plugins/monitoring/server/lib/elasticsearch_settings/find_reason.js

Actually I think the best solution would be to remove that if condition, and just let the checks happen

@chrisronline What do you think?

Sounds good to me as long as it solves this problem and doesn't introduce any additional issues, 👍

Can we add a test for this functionality? Verify it works as expected, verify it works despite the value of skip_unavailable?

I don't think this test will be very useful, since we don't have any skip_unavailable conditions/logic here.

The only test that makes sense to me is around getData in https://github.com/elastic/kibana/pull/44800/files#diff-87ec11d43f1e502792408786cf726eb6R32 and different states/reasons it could yield. WDYT?

I guess I was thinking a test for this because I had the thought to add a .skip_unavailable check, which you correctly mentioned was not necessary and would actually cause a problem. I don't feel strongly about it either way - just a thought