services: filter by labels ux enhancement

[Forfold]

• Identified the issue which this PR solves.
• Read the CONTRIBUTING document.
• Code builds clean without any errors or warnings.
• Added appropriate tests for any new functionality.
• All new and existing tests passed.
• Added comments in the code, where necessary.
• Ran make check to catch common errors. Fixed any that came up.

Description:
This PR allows individuals to filter services by labels with more ease. When a user selects a label key, it will show all services for that key (and populate the search field with the generated search query). From there, they can then select a label value to further refine those results.

Out of Scope:

• Allowing to search for service names/descriptions while simultaneously filtering by labels.

Screenshots:

Describe any introduced user-facing changes:

• Search bar moved from toolbar to just above the list for desktop.
• New filter menu visible when on the service's list page.

Describe any introduced API changes:

• Adds uniqueValues option to the labels query. This acts similarly to the preexisting option uniqueKeys.

Additional Info:

• Fixes some rendering issues present in PaginatedList.

[dctalbot]

Is there a more graceful way to handle these errors? Even if we just slice out "GraphQL: Error" I think it would make for a better UX

e.g.
"The label key's prefix must xyz..."
"The label key's prefix and suffix must xyz..."

[Forfold]

@dctalbot That's something currently app-wide that I've brought up a few times (stripping out ugly stuff from error messages). It's something I definitely want to to do, but where we do it would affect the errors app-wide, so it would be best to scope that to its own issue. We could certainly hard code it for this particular section to not show "Graphql", but I think it would be best to wait until we can follow through.

[mastercactapus]

@Auchindoun @dctalbot Doesn't that message have a field name prop? It should show up as an inline validation error on the key field/select, rather than the value.

[Forfold]

@mastercactapus I wasn't aware of any queries we have having field name errors. I thought this was just in our mutations.

[mastercactapus]

@Auchindoun all errors from the validation package will have a field name set, even if it's a query

[Forfold]

Looks like all errors are rendered through

    let noOptionsMessage = 'No options'
    if (optionsError) {
      noOptionsMessage = (
        <ErrorMessage value={optionsError.message || optionsError} />
      )
    }

and not using a FormHelper to display under the field. This applies to all of our QuerySelects, not just labels.

The above will show in the options field if there is an invalid key being set as the variable. (e.g. someone types "asdf=" in the search field, opens the filter container, opens the value select, and tries to start typing. This can also happen if the user directly inputs "asdf=" as the search parameter in the URL box. There's no real way around it with current implementation, as we pass the value of the key to the select as is, since we don't perform any client side validation in instances such as these.

Alternatively, we can always have the key's value in the select field default to null, so we would never get in this situation.

[mastercactapus]

Just a couple of small things left code wise.

Functionally, the filter icon seems to break on widescreen until refresh if the mobile version is rendered (try resizing the window to mobile and back, then clicking on it). It might be a React useRef bug though, not sure that it's anything we're doing wrong, it looks like it should work.

Can anyone else reproduce?

[Forfold]

(left: query, right: result)

Searching for something without a / in the labelKeys query does not return an error, and instead shows no results, which is why we are not seeing an error on the Key query but rather on the Value result set.

[mastercactapus]

@Auchindoun Ah, that makes sense. We'll need to look for a key error on the value select and raise it to the key field so it shows up correctly for the user.

Though, alternatively (and maybe a better route), we could consider if we really even need to validate it's a key -- we could validate it as "Search" input inside the SearchValues method. Hard LabelKey validation only really needs to be done if we're inserting new data to the DB. For queries, we just need to make sure it's safe to send as a parameter.

That would get rid of the odd error state and if it's an "invalid" key, you would just get no results, which should work fine.

[Forfold]

look for a key error on the value select and raise it to the key field so it shows up correctly for the user

I don't know if this should be the correct approach, as the key will have already been in an error state for some time, and a random error message would show in a field different from what the user is currently typing in.

Could you elaborate on your second thought? Not sure I know what you mean. What type of validation do you want to add to the key select query?

[mastercactapus]

For the first, we'd have to have some way of passing the error from the value field to the key field -- which feels messy.

For the second, we'd just change this to valiate.Search https://github.com/target/goalert/blob/service-search-labels/label/searchvalues.go#L58

Since there's actually no requirement that it be a key (it's read-only and only needs to be safe text). That also makes it use the same validation as the key search so there would never be a discrepancy.

[Forfold]

@mastercactapus Gotcha. I'll add that validation in there and see how it plays out in the UI

[Forfold]

So how it would play out right now is that they would just see no results if they were to do foo= int the URL or search field, then try to change the label value. All in agreeance? In the URL bar it would show on the main page that the query is already invalid, so they would see right away that there's an issue, and from the search field it would just say no results (potential bug there? do we want to validate services' search as LabelKey if = is present? @mastercactapus)

From URL:

From Search:

[mastercactapus]

@Auchindoun I think that works. If it's a valid key or not, it's ok. If it's a valid key with no results, or and invalid one with no results, I think we can display it the same.

It might be worth making the same validation change to service search so we don't show an error for invalid keys when searching. Validating the key really only needs to happen when we're putting data in the DB. But if they want to search for the key "foo" they will just get no results since there isn't a key named "foo".

---

It's like if you entered testing site:invalid_domain into google, invalid_domain can't actually be a domain, but it will still do the search without telling you you're wrong and just return no results

[Forfold]

Sounds good. I'll update validation on both so the same result set is always expected no matter the source