Skip to content

Add a NOT IN operator for KeywordList search attributes - #3961

Open
rossedfort wants to merge 1 commit into
mainfrom
rossedfort/fe-613-add-not-operator-for-keywordlist-search-attributes
Open

rossedfort wants to merge 1 commit into
mainfrom
rossedfort/fe-613-add-not-operator-for-keywordlist-search-attributes

Conversation

@rossedfort

Copy link
Copy Markdown
Contributor

Description & motivation 💭

KeywordList search attributes (BuildIds, TemporalChangeVersion, and any custom one) could be filtered with in, =, !=, is null and is not null, but there was no way to express "this list contains none of these values". != is not a substitute — it compares a single value, so excluding three build IDs meant three separate chips.

Every layer below the UI already handled not in:

  • 'not in' is in the conditionals grammar list in src/lib/utilities/is.ts, and isInConditional matches it.
  • tokenize.ts has a six-character lookahead written specifically to lex it, already covered by a test.
  • formatValue in filter-workflow-query.ts passes the parenthesized literal through unquoted for any in-style conditional.
  • The generic branch of toListWorkflowFilters stores the whole ("a", "b") token as the value and captures not in as the conditional.

So the fix is one entry in listConditionalOptions. The chip's value editor already picks ChipInput vs. a plain input via isInConditional, and applyChanges already re-serializes chips to ("a", "b") for it.

The query serializes unspaced, matching how in is emitted today: `BuildIds`not in("a", "b").

Confirmed server-side support before building this: in common/persistence/visibility/store/query/converter.go, supportedKeywordListOperators is exactly EqualStr, NotEqualStr, InStr, NotInStr. (The List Filter docs page doesn't mention NOT IN, but the converter accepts it.)

Scoped to KeywordList only. Keyword attributes support NOT IN server-side too, but they have no In option in the UI today, so adding Not In there alone would be inconsistent.

The hardcoded English 'In' label moved to a translation key alongside the new 'Not In', so the pair stays consistent.

Testing 🧪

How was this tested 👻

  • Manual testing
  • E2E tests added
  • Unit tests added

Unit: not in serialization (alone and AND-joined) in filter-workflow-query.test.ts; parsing `CustomKeywordListField`not in("Hello", "World") back into a filter (alone and joined) in to-list-workflow-filters.test.ts.

E2E: a KeywordList search attributes block registering a KeywordList custom attribute, covering the Not In flow, the unchanged In flow, and a round trip through the raw query box.

Full unit suite and the integration suite pass; cold svelte-check reports 0 errors; lint:ci is clean.

Manual testing has not been done — nothing here has exercised a real NOT IN query against Elasticsearch. Worth a pass before merge.

Steps for others to test: 🚶🏽‍♂️🚶🏽‍♀️

  1. Run the UI against a server with a KeywordList search attribute populated (BuildIds or TemporalChangeVersion).
  2. On the workflows list, Add Filter → pick the KeywordList attribute.
  3. Choose Not In, enter two values, Apply.
  4. The URL should read `BuildIds`not in("a", "b") and the results should exclude matching workflows.
  5. Reload — the chip should rebuild with Not In still selected.
  6. Toggle the raw query box — it should show the same string and re-accept it.

Issue(s) closed

FE-613

@rossedfort
rossedfort requested a review from a team as a code owner September 29, 2026 19:50
@vercel

vercel Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
holocene Ready Ready Preview Sep 29, 2026 7:51pm UTC

Request Review

@andrewzamojc andrewzamojc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

const listConditionalOptions = $derived([
{ value: 'in', label: 'In' },
{ value: 'in', label: translate('common.in') },
{ value: 'not in', label: translate('common.not-in') },

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is already working and you're just adding it to the FE? All the other files are tests.

This branch was successfully deployed

1 active deployment
Preview — 8e1db7d9 Deployed Sep 29, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants