synthetic-monitoring-app icon indicating copy to clipboard operation
synthetic-monitoring-app copied to clipboard

chore: enforce tenant limits on the frontend

Open rdubrock opened this issue 10 months ago • 3 comments

I'm a little unsure where the best place to gate access based on limits is. I definitely don't want it to be how it currently is (when you submit a form after filling it out), but there are other options than what I've done. Opinions welcome.

Fixes https://github.com/grafana/synthetic-monitoring/issues/49

No limits reached: Screenshot 2024-04-03 at 14 07 38

Overall limit reached: Screenshot 2024-04-03 at 13 13 38

Scripted limit reached: Screenshot 2024-04-03 at 13 12 39

rdubrock avatar Apr 03 '24 22:04 rdubrock

I know it is minor but I'd rather remove the LabelField element and have a retry button about trying to refetch the tenant limits so we can decipher the amount of labels they can have rather than us guessing.

Yeah I waffled on this. I'd rather not block them doing what they want. I think maybe better to show a warning with the retry button but still default. The defaults are the minimum quantity of custom labels available, and it's only going to be a tiny minority of checks that need more than the minimum anyways. So I'm more worried about cutting off the path to getting the check submitted than the path to attach more than the minimum quantity of labels

It's likely moot either way, because if the limits endpoint is busted the submit endpoint probably is too, but in the eventuality I'd rather reduced functionality than none

rdubrock avatar Apr 08 '24 18:04 rdubrock

Error state:

Screenshot 2024-04-08 at 11 46 33

rdubrock avatar Apr 08 '24 20:04 rdubrock

I'd prefer this test if we either explicitly set the feature toggle to false with a comment or we just removed it altogether

Don't want to dig in my heels too much, but setting the flag to false to me is testing that the feature flag is hiding the card (which we also have a test for). Setting the flag to true is actually the scenario users are going to experience, so I'd rather test what's going to happen in the wild. I added an explanatory note. Not a hill I'm trying to die on, though, if that doesn't cut it we can delete it no problem

rdubrock avatar Apr 09 '24 19:04 rdubrock