This was one of the limitations I ran into with the new API endpoint: you can't make a single request for every repo key that the user adds to their workspaces config. If your config has the repo URL keys [repo_url, alt_repo_url], then a request like this:

owner:me param:"repo_url=url.com" param:"alt_repo_url=url.com"

Won't work, because it will search for workspaces that have both keys, even though realistically, a workspace will only ever have one of them

It's not a huge deal – the previous logic was using Promise.allSettled, too – but it's not making the API logic that much faster

Yea, this is just an unfortunate consequence of our search filtering. We have to make opinions, and cannot support both union and intersection type searches easily at the moment.

Our search filter is very limited at present, and there is no effort to improve it in it's current state. It's just too much of a headache is the answer atm.

Another idea I had awhile back worth peaking at: coder/coder#12706

^^ What steven said
I chatted with Steven a little bit about this constraint and I guess our current search filtering cannot support both intersection and union searches. To implement both we'd basically have to duplicate all of this logic, which is already really long.

There's not a shorter syntax because we use SQLc so we must use CASE statements. Here is an open discussion we can return to in the future if this ever becomes more problematic.

Anyway, all this to say I think what you have is great!

Removed the spaces (1) to account for the mock server logic better, and (2) to account for the fact that the actual Coder API doesn't expect spaces like this, either

are key and stringUrl guaranteed to be strings as opposed to null or undefined?

Luckily, they're both guaranteed to be defined

With key, we're getting the values through .map. That doesn't guarantee that we'll have a non-empty array, but it does mean that the mapper callback will only be called if we have a defined element to feed into it. That's one of the benefits of the array methods and for/of over traditional i loops – they can guarantee no null-ish values, because they're going through JS's iterator protocol

And as for stringUrl, it's wonky, but that's also guaranteed to be of type string. If we had this:

const patchedRequest = {
  ...request,
  q: appendParamToQuery(request.q, key, config.repoUrl),
};

config.repoUrl would be type string | undefined. But TS type narrowing helps get around that:

// Rule out that config.repoUrl is not undefined; TS is smart
// enough to shrink size down to just string for all code after
// this if statement
if (config.repoUrl === undefined) {
  return { workspaces: [], count: 0 };
}

// Capture the value after we've proven it's a string only. Because
// this is a const and can't be reassigned, TS knows that undefined
// can never be assigned to this version of the value
const stringUrl = config.repoUrl;

// Normally, the type of a union value would "reset" when you
// have a different callback, because TS can't look at the code and
// make guarantees about whether the value will change before the
// callback runs. But as long as we use the "captured" version of
// the value, it will always be a string and can't be reset
const responses = /* */

But as long as we use the "captured" version of the value, it will always be a string and can't be reset

I see your comment above now. Thank you for the explanation!

Is it worth check that these don't contain empty strings?

Oh, good call. Let me fix that really quick