[Selection input] Filter out kind tags from tags + Remove contains ma…

…tch filter as first option in empty case (#27385) ## Summary & Motivation Kind tags already show up under the `kind:` filter, so removing them from showing up under the `tag:` filter. ## How I Tested These Changes locally, didn't add a jest test for the getAttributes function because I'm in the middle of refactoring all of this logic to be more flexible/robust. The SelectionAutoComplete jest test covers the second part of this PR ![image](https://github.com/user-attachments/assets/aafe7b7d-5744-498a-ba84-698717ceafa8)
dagster-io · Feb 3, 2025 · efd6a33 · efd6a33 · github-actions · Feb 3, 2025
1 parent 1bee8d6
commit efd6a33
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 149 deletions.
diff --git a/js_modules/dagster-ui/packages/ui-core/src/asset-selection/input/util.ts b/js_modules/dagster-ui/packages/ui-core/src/asset-selection/input/util.ts
@@ -1,5 +1,6 @@
 import {assertUnreachable} from '../../app/Util';
 import {AssetGraphQueryItem} from '../../asset-graph/useAssetGraphData';
+import {isKindTag} from '../../graph/KindTags';
 import {buildRepoPathForHuman} from '../../workspace/buildRepoAddress';
 
 export const getAttributesMap = (assets: AssetGraphQueryItem[]) => {
@@ -13,6 +14,9 @@ export const getAttributesMap = (assets: AssetGraphQueryItem[]) => {
   assets.forEach((asset) => {
     assetNamesSet.add(asset.name);
     asset.node.tags.forEach((tag) => {
+      if (isKindTag(tag)) {
+        return;
+      }
       if (tag.key && tag.value) {
         // We add quotes around the equal sign here because the auto-complete suggestion already wraps the entire value in quotes.
         // So wer end up with tag:"key"="value" as the final suggestion

diff --git a/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionAutoCompleteVisitor.ts b/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionAutoCompleteVisitor.ts
@@ -30,7 +30,7 @@ import {
 const DEFAULT_TEXT_CALLBACK = (value: string) => value;
 
 // set to true for debug output if desired
-const DEBUG = true;
+const DEBUG = false;
 
 export type Suggestion =
   | {
@@ -99,7 +99,6 @@ export class SelectionAutoCompleteVisitor extends BaseSelectionVisitor {
     this.functions = functions;
     this.nameBase = nameBase;
     this.attributesWithNameBase = [
-      `${this.nameBase}_substring`,
       this.nameBase,
       ...Object.keys(attributesMap).filter((name) => name !== this.nameBase),
     ];

diff --git a/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInput.tsx b/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInput.tsx
@@ -330,7 +330,7 @@ export const SelectionAutoCompleteInput = ({
   useResizeObserver(inputRef, adjustHeight);
 
   return (
-    <div onBlur={onBlur}>
+    <div onBlur={onBlur} style={{width: '100%'}}>
       <Popover
         content={
           <div ref={hintContainerRef} onKeyDown={handleKeyDown}>

diff --git a/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInputAutoCompleteResults.tsx b/js_modules/dagster-ui/packages/ui-core/src/selection/SelectionInputAutoCompleteResults.tsx
@@ -101,7 +101,7 @@ export const SelectionInputAutoCompleteResults = React.memo(
                         text={<SuggestionItem suggestion={result} />}
                         active={index === selectedIndex}
                         onClick={() => onSelect(result)}
-                        onMouseEnter={() => setSelectedIndex({current: index})}
+                        onMouseMove={() => setSelectedIndex({current: index})}
                       />
                     </div>
                   </Row>
@@ -180,6 +180,7 @@ const SuggestionItem = ({suggestion}: {suggestion: Suggestion}) => {
   } else if (suggestion.type === 'parenthesis') {
     icon = 'curly_braces';
     label = 'Parenthesis';
+    value = suggestion.text;
   } else if (suggestion.type === 'function') {
     if (suggestion.displayText === 'roots()') {
       label = 'Roots';
@@ -194,16 +195,25 @@ const SuggestionItem = ({suggestion}: {suggestion: Suggestion}) => {
 
     icon = attributeToIcon[suggestion.displayText as Attribute];
   } else if (suggestion.type === 'attribute-with-value') {
-    const firstColon = suggestion.displayText.indexOf(':');
-    const attributeKey = suggestion.displayText.slice(0, firstColon);
-    const attributeValue = suggestion.displayText.slice(firstColon + 1);
-    label = (
-      <Box flex={{direction: 'row', alignItems: 'center', gap: 2}}>
-        <MonoSmall color={Colors.textLight()}>{attributeKey}:</MonoSmall>
-        <MonoSmall>{attributeValue}</MonoSmall>
-      </Box>
-    );
-    value = null;
+    // hacky approach to extract the substring filter for now..., upcoming refactor will have more robust logic.
+    const substringMatch = /^([a-zA-Z]+)_substring:(.+)$/.exec(suggestion.text);
+    if (substringMatch) {
+      const nameBase = substringMatch[1]!;
+      label = `${nameBase[0]!.toUpperCase() + nameBase.slice(1)} contains ${substringMatch[2]}`;
+      icon = 'magnify_glass_checked';
+      value = suggestion.displayText;
+    } else {
+      const firstColon = suggestion.displayText.indexOf(':');
+      const attributeKey = suggestion.displayText.slice(0, firstColon);
+      const attributeValue = suggestion.displayText.slice(firstColon + 1);
+      label = (
+        <Box flex={{direction: 'row', alignItems: 'center', gap: 2}}>
+          <MonoSmall color={Colors.textLight()}>{attributeKey}:</MonoSmall>
+          <MonoSmall>{attributeValue}</MonoSmall>
+        </Box>
+      );
+      value = null;
+    }
   } else if (suggestion.type === 'attribute-value') {
     label = suggestion.displayText;
     value = null;

diff --git a/js_modules/dagster-ui/packages/ui-core/src/selection/__tests__/SelectionAutoComplete.test.ts b/js_modules/dagster-ui/packages/ui-core/src/selection/__tests__/SelectionAutoComplete.test.ts
@@ -143,13 +143,6 @@ describe('createAssetSelectionHint', () => {
     // empty case
     expect(testAutocomplete('|')).toEqual({
       list: [
-        {
-          displayText: 'key_substring:',
-          text: 'key_substring:',
-          type: 'attribute',
-          attributeName: 'key_substring',
-          nameBase: true,
-        },
         {
           displayText: 'key:',
           text: 'key:',
@@ -263,13 +256,6 @@ describe('createAssetSelectionHint', () => {
   it('should handle traversal operators correctly', () => {
     expect(testAutocomplete('+|')).toEqual({
       list: [
-        {
-          displayText: 'key_substring:',
-          text: 'key_substring:',
-          type: 'attribute',
-          attributeName: 'key_substring',
-          nameBase: true,
-        },
         {
           displayText: 'key:',
           text: 'key:',
@@ -332,13 +318,6 @@ describe('createAssetSelectionHint', () => {
     expect(testAutocomplete('+ |')).toEqual({
       from: 2,
       list: [
-        {
-          displayText: 'key_substring:',
-          text: 'key_substring:',
-          type: 'attribute',
-          attributeName: 'key_substring',
-          nameBase: true,
-        },
         {
           displayText: 'key:',
           text: 'key:',
@@ -392,13 +371,6 @@ describe('createAssetSelectionHint', () => {
     expect(testAutocomplete('+|')).toEqual({
       from: 1,
       list: [
-        {
-          displayText: 'key_substring:',
-          text: 'key_substring:',
-          type: 'attribute',
-          attributeName: 'key_substring',
-          nameBase: true,
-        },
         {
           displayText: 'key:',
           text: 'key:',
@@ -473,13 +445,6 @@ describe('createAssetSelectionHint', () => {
   it('should handle incomplete "not" expressions', () => {
     expect(testAutocomplete('not|')).toEqual({
       list: [
-        {
-          text: ' key_substring:',
-          displayText: 'key_substring:',
-          type: 'attribute',
-          attributeName: 'key_substring',
-          nameBase: true,
-        },
         {
           text: ' key:',
           displayText: 'key:',
@@ -533,13 +498,6 @@ describe('createAssetSelectionHint', () => {
 
     expect(testAutocomplete('not |')).toEqual({
       list: [
-        {
-          text: 'key_substring:',
-          displayText: 'key_substring:',
-          type: 'attribute',
-          attributeName: 'key_substring',
-          nameBase: true,
-        },
         {
           text: 'key:',
           displayText: 'key:',
@@ -595,13 +553,6 @@ describe('createAssetSelectionHint', () => {
   it('should handle incomplete and expressions', () => {
     expect(testAutocomplete('key:"asset1" and |')).toEqual({
       list: [
-        {
-          text: 'key_substring:',
-          displayText: 'key_substring:',
-          type: 'attribute',
-          attributeName: 'key_substring',
-          nameBase: true,
-        },
         {
           text: 'key:',
           displayText: 'key:',
@@ -658,13 +609,6 @@ describe('createAssetSelectionHint', () => {
   it('should handle incomplete or expressions', () => {
     expect(testAutocomplete('key:"asset1" or |')).toEqual({
       list: [
-        {
-          text: 'key_substring:',
-          displayText: 'key_substring:',
-          type: 'attribute',
-          attributeName: 'key_substring',
-          nameBase: true,
-        },
         {
           text: 'key:',
           displayText: 'key:',
@@ -750,13 +694,6 @@ describe('createAssetSelectionHint', () => {
       testAutocomplete('sinks(key_substring:"FIVETRAN/google_ads/ad_group_history" or |)'),
     ).toEqual({
       list: [
-        {
-          text: 'key_substring:',
-          displayText: 'key_substring:',
-          type: 'attribute',
-          attributeName: 'key_substring',
-          nameBase: true,
-        },
         {
           text: 'key:',
           displayText: 'key:',
@@ -816,13 +753,6 @@ describe('createAssetSelectionHint', () => {
     ).toEqual({
       list: [
         // Inserts a space before the string
-        {
-          text: ' key_substring:',
-          displayText: 'key_substring:',
-          type: 'attribute',
-          attributeName: 'key_substring',
-          nameBase: true,
-        },
         {
           text: ' key:',
           displayText: 'key:',
@@ -945,14 +875,6 @@ describe('createAssetSelectionHint', () => {
       testAutocomplete('key:"test" or key_substring:"FIVETRAN/google_ads/ad_group_history"+ or |'),
     ).toEqual({
       list: [
-        // Inserts a space before the string
-        {
-          text: 'key_substring:',
-          displayText: 'key_substring:',
-          type: 'attribute',
-          attributeName: 'key_substring',
-          nameBase: true,
-        },
         {
           text: 'key:',
           displayText: 'key:',
@@ -1009,14 +931,6 @@ describe('createAssetSelectionHint', () => {
   it('suggestions inside parenthesized expression', () => {
     expect(testAutocomplete('(|)')).toEqual({
       list: [
-        // Inserts a space before the string
-        {
-          text: 'key_substring:',
-          displayText: 'key_substring:',
-          type: 'attribute',
-          attributeName: 'key_substring',
-          nameBase: true,
-        },
         {
           text: 'key:',
           displayText: 'key:',
@@ -1073,13 +987,6 @@ describe('createAssetSelectionHint', () => {
   it('suggestions outside parenthesized expression before', () => {
     expect(testAutocomplete('|()')).toEqual({
       list: [
-        {
-          text: 'key_substring:',
-          displayText: 'key_substring:',
-          type: 'attribute',
-          attributeName: 'key_substring',
-          nameBase: true,
-        },
         {
           text: 'key:',
           displayText: 'key:',
@@ -1225,13 +1132,6 @@ describe('createAssetSelectionHint', () => {
   it('makes suggestions for IncompleteExpression inside of the ParenthesizedExpression', () => {
     expect(testAutocomplete('(key:tag and |)')).toEqual({
       list: [
-        {
-          displayText: 'key_substring:',
-          text: 'key_substring:',
-          type: 'attribute',
-          attributeName: 'key_substring',
-          nameBase: true,
-        },
         {
           displayText: 'key:',
           text: 'key:',
@@ -1286,13 +1186,6 @@ describe('createAssetSelectionHint', () => {
 
     expect(testAutocomplete('(key:tag and|)')).toEqual({
       list: [
-        {
-          displayText: 'key_substring:',
-          text: ' key_substring:',
-          type: 'attribute',
-          attributeName: 'key_substring',
-          nameBase: true,
-        },
         {
           displayText: 'key:',
           text: ' key:',
@@ -1347,13 +1240,6 @@ describe('createAssetSelectionHint', () => {
 
     expect(testAutocomplete('(key:tag and| )')).toEqual({
       list: [
-        {
-          displayText: 'key_substring:',
-          text: ' key_substring:',
-          type: 'attribute',
-          attributeName: 'key_substring',
-          nameBase: true,
-        },
         {
           displayText: 'key:',
           text: ' key:',
@@ -1428,13 +1314,6 @@ describe('createAssetSelectionHint', () => {
     ).toEqual({
       from: 35,
       list: [
-        {
-          displayText: 'key_substring:',
-          text: 'key_substring:',
-          type: 'attribute',
-          attributeName: 'key_substring',
-          nameBase: true,
-        },
         {
           displayText: 'key:',
           text: 'key:',
@@ -1563,13 +1442,6 @@ describe('createAssetSelectionHint', () => {
     ).toEqual({
       from: 54,
       list: [
-        {
-          attributeName: 'key_substring',
-          displayText: 'key_substring:',
-          text: 'key_substring:',
-          type: 'attribute',
-          nameBase: true,
-        },
         {
           displayText: 'key:',
           text: 'key:',
@@ -1598,13 +1470,6 @@ describe('createAssetSelectionHint', () => {
     expect(testAutocomplete('key:"value"+ or tag:"value"+ and owner:"owner" and |')).toEqual({
       from: 51,
       list: [
-        {
-          displayText: 'key_substring:',
-          text: 'key_substring:',
-          type: 'attribute',
-          attributeName: 'key_substring',
-          nameBase: true,
-        },
         {
           displayText: 'key:',
           text: 'key:',