Why did we addvals.Get(queryParam) == ""? The empty string is a valid value for parsing params here. If it is not valid for your use case, we should solve it somewhere else.

I think this could break something 🤔

Yeah I had mixed feelings about this; it felt surprising to me that I could writeRequired("client_id") and it could come through empty. My thinking was that if I want to require a query param I also want a value.

I did check existing usages; it is used forreconnect on the terminal (which is invalid if blank) and forstart_time andend_time on an insights endpoint which also cannot be blank (it will fail to be parsed into a time).

But at the same time I see the distinction between required and empty. An empty query value is probably only useful as a boolean where the presence of the key meanstrue, and I could see requiring atrue for something like?acceptTerms or something like that before you can use the endpoint.

So let me rename this toRequiredNotEmpty, that will leave room forRequired if we do need that some day.

Some new ones popped up after I merged main:template_id,workspace_id, andagent_id on some endpoints, they all make database queries with these values so I think they would just return "no rows" or a 404 or whatever, so this change will make it return "invalid query params, template_id is required" instead which I think is fair, but it does change the semantics a little from a 404 to a 400.

I wonder if we should hardcode this to/login/oauth2/me/tokens for now to allow us supporting revoking other user's in the future.

Oh really good point, let me change it now real quick.

Oh right I had it here to have the post/delete paradigm on the same endpoint. Maybe I should actually move this back to/api/v2 since it is our own thing, not part of the whole oauth flow.

This is not an oauth2 spec endpoint right?

Eh just keep it here and comment that it is not oauth 2 spec. If you feel this has to be a static endpoint though, then just comment why it should be. I'll defer to your judgement

Yup exactly, it is just our own thing and not part of the spec. I do not feel strongly about it, just felt kinda right to have posts and deletes on the same endpoint. Hmmm lemme give it some thought!