[Monitoring] Fix rison error

[chrisronline]

Fixes #83212

This is a regression introduced by #78979 and the fix is to ensure the cluster_uuid is contained in quotes.

[elasticmachine]

Pinging @elastic/stack-monitoring (Team:Monitoring)

[chrisronline]

@elasticmachine merge upstream

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: dd24b40

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
monitoring	952.1KB	952.2KB	+58.0B

History

• 💚 Build #89397 succeeded 85b2af0

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

[igoristic]

Technically rison should already cast strings for us, I don't know why we even have to do this

But if we must, we should first check to see if it's already wrapped in quotes (or, you'll keep adding quotes to the value over and over again). So we should also add the following checks like this:

Suggested change

	if (globalStateKey === 'cluster_uuid') {
	if (globalStateKey === 'cluster_uuid' && value.charAt() !== '"' && value.charAt() !== '\'') {

[chrisronline]

Where are you seeing the double quotes happening? I'm trying to reproduce it locally but can't seem to find the right UI flow

[igoristic]

Here is an example: https://demo.elastic.co/app/monitoring#/home

• Notice how clicking on the first cluster has no quotes ...(cluster_uuid:C7AppFTyQfWdUL3ldutLjQ,
• And clicking on the second cluster has single quotes ...(cluster_uuid:'0f0rwRiWTjaJPTrORm4PoA',

Since, we don't take "checking first" into account we run the risk of adding unwanted quotes

[chrisronline]

Oh, I'm a little confused sorry. Why are you testing this PR on the demo site? Are you connecting your local Kibana instance to that cluster or something?

Locally, with two clusters setup, I get from clicking on one cluster:
https://localhost:5601/app/monitoring#/overview?_g=(ccs:Monitoring,cluster_uuid:'1sO4J1fWQEq8AU628IoBGw',refreshInterval:(pause:!t,value:10000),time:(from:now-1h,to:now))

Then, I see https://localhost:5601/app/monitoring#/overview?_g=(refreshInterval:(pause:!t,value:10000),time:(from:now-1h,to:now),cluster_uuid:'TpRkL2e-QVaJisp084RwzQ') when clicking the other.

I can see in the code how we could be double quoting it, but I can't seem to find the use case of it actually happening (so I can verify the fix works)

[igoristic]

Ah, I see it now "globalState" here is not actually the globalState we're use to, more of a way to override parameters (since we only use it in one place).

In which case, why not just pass the following cluster_uuid already as a quoted string here:

kibana/x-pack/plugins/monitoring/public/components/cluster/listing/listing.js

Line 82 in 3f97872

href={getSafeForExternalLink(`#/overview`, { cluster_uuid: cluster.cluster_uuid })}

so it'd look something like:

getSafeForExternalLink(`#/overview`, { cluster_uuid: `'${cluster.cluster_uuid}'` })

This way we don't have to check which key is which and how to cast it.

Might be more of nit/preference on my end, so I'm fine if you decide to keep it. Let me know and I'll approve it

[chrisronline]

Fair enough. I worry that requiring the consumer to provide the ' will lead to us forgetting to do that. I'm not sure if there is a better way right now though.

[igoristic]

Looks good 👍