Skip to content

Commit 93f0e8f

Browse files
laurakwhitlaurakwhit
authored
Fix custom search attributes inputs for different attribute types (#3215)
* Fix sa input reactivity based on sa type * Keep isDisabled synced across inputs on sa change * Add custom sa tests to start a workflow * Trigger effect only when the local value state changes * Add untrack comment
1 parent 954c647 commit 93f0e8f

File tree

3 files changed

+151
-27
lines changed

3 files changed

+151
-27
lines changed

‎src/lib/components/workflow/add-search-attributes.svelte‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
};
3535
3636
const searchAttributes = $derived(Object.keys($customSearchAttributes));
37-
const attributes = $derived(attributesToAdd);
37+
const attributes = $derived([...attributesToAdd]);
3838
3939
const onRemove = (attribute: string) => {
4040
attributesToAdd = attributesToAdd.filter((a) => a.label !== attribute);

‎src/lib/components/workflow/search-attribute-input/index.svelte‎

Lines changed: 41 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
<script lang="ts">
2+
import { untrack } from 'svelte';
3+
24
import Button from '$lib/holocene/button.svelte';
35
import ChipInput from '$lib/holocene/input/chip-input.svelte';
46
import Input from '$lib/holocene/input/input.svelte';
@@ -12,7 +14,6 @@
1214
type SearchAttributeSchema,
1315
type SearchAttributesSchema,
1416
} from '$lib/stores/search-attributes';
15-
import type { SelectOptionValue } from '$lib/types/global';
1617
import { SEARCH_ATTRIBUTE_TYPE } from '$lib/types/workflows';
1718
1819
import DatetimeInput from './datetime-input.svelte';
@@ -26,26 +27,39 @@
2627
2728
let { attributes, attribute = $bindable(), onRemove, id }: Props = $props();
2829
29-
let label = $state<SelectOptionValue | undefined>(undefined);
30-
let _label = $derived(attribute.label || (label && label.toString()));
30+
let type = $state(attribute.type);
31+
let value = $state(attribute.value);
32+
let label = $state(attribute.label);
33+
34+
$effect(() => {
35+
const v = value;
36+
// effect should only re-run when the local value state changes
37+
// changes to attribute should not re-trigger the effect
38+
untrack(() => {
39+
attribute.value = v;
40+
});
41+
});
3142
32-
const isDisabled = (value: string) => {
33-
return !!attributes.find((a) => a.label === value);
43+
const isDisabled = (v: string) => {
44+
return label !== v && !!attributes.find((a) => a.label === v);
3445
};
3546
3647
const getType = (attr: string) => $customSearchAttributes[attr];
3748
3849
const handleAttributeChange = (attr: string) => {
39-
attribute.label = attr;
40-
const type = getType(attr);
50+
const previousType = type;
51+
const newType = getType(attr);
52+
53+
label = attr;
54+
type = newType;
4155
42-
if (type === SEARCH_ATTRIBUTE_TYPE.KEYWORDLIST) {
43-
attribute.value = [];
44-
} else if (attribute.type !== type) {
45-
attribute.value = null;
56+
if (newType === SEARCH_ATTRIBUTE_TYPE.KEYWORDLIST) {
57+
value = [];
58+
} else if (previousType !== newType) {
59+
value = null;
4660
}
4761
48-
attribute.type = type;
62+
attribute = { label, value, type };
4963
};
5064
</script>
5165

@@ -59,7 +73,7 @@
5973
data-testid="search-attribute-select"
6074
label={translate('workflows.custom-search-attribute')}
6175
placeholder={translate('workflows.select-attribute')}
62-
bind:value={_label}
76+
bind:value={label}
6377
onChange={handleAttributeChange}
6478
>
6579
{#each $customSearchAttributeOptions as { value, label, type } (value)}
@@ -72,48 +86,49 @@
7286
<Button
7387
variant="ghost"
7488
leadingIcon="close"
89+
data-testid="search-attribute-close-button"
7590
class="mt-6 w-10 rounded-full sm:hidden"
76-
on:click={() => onRemove(attribute.label)}
91+
on:click={() => onRemove(label)}
7792
/>
7893
</div>
79-
{#if attribute.type === SEARCH_ATTRIBUTE_TYPE.BOOL}
94+
{#if type === SEARCH_ATTRIBUTE_TYPE.BOOL}
8095
<Select
8196
label={translate('common.value')}
8297
id="attribute-value-{id}"
83-
bind:value={attribute.value}
98+
bind:value
8499
>
85100
<Option value={true}>{translate('common.true')}</Option>
86101
<Option value={false}>{translate('common.false')}</Option>
87102
</Select>
88-
{:else if attribute.type === SEARCH_ATTRIBUTE_TYPE.DATETIME}
89-
<DatetimeInput bind:value={attribute.value} />
90-
{:else if attribute.type === SEARCH_ATTRIBUTE_TYPE.INT || attribute.type === SEARCH_ATTRIBUTE_TYPE.DOUBLE}
103+
{:else if type === SEARCH_ATTRIBUTE_TYPE.DATETIME}
104+
<DatetimeInput bind:value />
105+
{:else if type === SEARCH_ATTRIBUTE_TYPE.INT || type === SEARCH_ATTRIBUTE_TYPE.DOUBLE}
91106
<div>
92107
<NumberInput
93108
label={translate('common.value')}
94109
id="attribute-value-{id}"
95-
valid={attribute.value < Number.MAX_SAFE_INTEGER}
110+
valid={value < Number.MAX_SAFE_INTEGER}
96111
hintText="Number is too large"
97-
bind:value={attribute.value}
112+
bind:value
98113
max={Number.MAX_SAFE_INTEGER}
99114
/>
100115
</div>
101-
{:else if attribute.type === SEARCH_ATTRIBUTE_TYPE.KEYWORDLIST}
116+
{:else if type === SEARCH_ATTRIBUTE_TYPE.KEYWORDLIST}
102117
<ChipInput
103118
label={translate('common.value')}
104119
id="attribute-value-{id}"
105-
bind:chips={attribute.value}
120+
bind:chips={value}
106121
class="w-full"
107122
removeChipButtonLabel={(chip) =>
108123
translate('workflows.remove-keyword-label', { keyword: chip })}
109124
/>
110-
{:else if attribute.type === SEARCH_ATTRIBUTE_TYPE.TEXT || attribute.type === SEARCH_ATTRIBUTE_TYPE.KEYWORD || attribute.type === SEARCH_ATTRIBUTE_TYPE.UNSPECIFIED}
125+
{:else if type === SEARCH_ATTRIBUTE_TYPE.TEXT || type === SEARCH_ATTRIBUTE_TYPE.KEYWORD || type === SEARCH_ATTRIBUTE_TYPE.UNSPECIFIED}
111126
<Input
112127
label={translate('common.value')}
113128
data-testid="custom-search-attribute-value"
114129
id="attribute-value-{id}"
115130
class="grow"
116-
bind:value={attribute.value}
131+
bind:value
117132
/>
118133
{:else}
119134
<Input
@@ -131,6 +146,6 @@
131146
leadingIcon="close"
132147
data-testid="search-attribute-close-button"
133148
class="mt-6 w-10 rounded-full max-sm:hidden"
134-
on:click={() => onRemove(attribute.label)}
149+
on:click={() => onRemove(label)}
135150
/>
136151
</div>

‎tests/integration/start-a-workflow.spec.ts‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { expect, test } from '@playwright/test';
22

3+
import { SEARCH_ATTRIBUTE_TYPE } from '$src/lib/types/workflows';
34
import {
45
mockGlobalApis,
56
mockNamespaceApi,
@@ -55,4 +56,112 @@ test.describe('Start a Workflow', () => {
5556
await expect(page.getByTestId('start-workflow-button')).toBeEnabled();
5657
});
5758
});
59+
60+
test.describe('Custom Search Attributes', () => {
61+
test.beforeEach(async ({ page }) => {
62+
await mockSettingsApi(page, { StartWorkflowDisabled: false });
63+
await mockSearchAttributesApi(page, {
64+
customAttributes: {
65+
CustomBool: SEARCH_ATTRIBUTE_TYPE.BOOL,
66+
CustomKeyword: SEARCH_ATTRIBUTE_TYPE.KEYWORD,
67+
},
68+
});
69+
await page.goto(startWorkflowUrl);
70+
await page.getByText('More options').click();
71+
});
72+
73+
test('adds a search attribute and shows correct input for Bool type', async ({
74+
page,
75+
}) => {
76+
await page.getByTestId('add-search-attribute-button').click();
77+
78+
const attributeSelect = page.getByTestId(
79+
'search-attribute-select-button',
80+
);
81+
await attributeSelect.click();
82+
await page.getByRole('option', { name: 'CustomBool' }).click();
83+
84+
await expect(page.getByTestId('attribute-value-0-button')).toBeVisible();
85+
});
86+
87+
test('updates value for Bool attribute', async ({ page }) => {
88+
await page.getByTestId('add-search-attribute-button').click();
89+
90+
const attributeSelect = page.getByTestId(
91+
'search-attribute-select-button',
92+
);
93+
await attributeSelect.click();
94+
await page.getByRole('option', { name: 'CustomBool' }).click();
95+
96+
const valueSelect = page.getByTestId('attribute-value-0-button');
97+
await valueSelect.click();
98+
await page.getByRole('option', { name: 'True' }).click();
99+
100+
await expect(page.locator('#attribute-value-0')).toHaveValue('True');
101+
});
102+
103+
test('switches input type when changing attribute selection', async ({
104+
page,
105+
}) => {
106+
await page.getByTestId('add-search-attribute-button').click();
107+
108+
const attributeSelect = page.getByTestId(
109+
'search-attribute-select-button',
110+
);
111+
await attributeSelect.click();
112+
await page.getByRole('option', { name: 'CustomKeyword' }).click();
113+
114+
await expect(
115+
page.getByTestId('custom-search-attribute-value'),
116+
).toBeVisible();
117+
118+
await attributeSelect.click();
119+
await page.getByRole('option', { name: 'CustomBool' }).click();
120+
121+
await expect(
122+
page.getByTestId('custom-search-attribute-value'),
123+
).toBeHidden();
124+
await expect(page.getByTestId('attribute-value-0-button')).toBeVisible();
125+
});
126+
127+
test('disables already selected attributes in other rows', async ({
128+
page,
129+
}) => {
130+
await page.getByTestId('add-search-attribute-button').click();
131+
132+
const firstSelect = page
133+
.getByTestId('search-attribute-select-button')
134+
.first();
135+
await firstSelect.click();
136+
await page.getByRole('option', { name: 'CustomBool' }).click();
137+
138+
await page.getByTestId('add-search-attribute-button').click();
139+
140+
const secondSelect = page
141+
.getByTestId('search-attribute-select-button')
142+
.nth(1);
143+
await secondSelect.click();
144+
145+
const customBoolOption = page.locator(
146+
'#search-attribute-1-select > li[role="option"]',
147+
{ hasText: 'CustomBool' },
148+
);
149+
await expect(customBoolOption).toHaveAttribute('aria-disabled', 'true');
150+
});
151+
152+
test('removes a search attribute row', async ({ page }) => {
153+
await page.getByTestId('add-search-attribute-button').click();
154+
await expect(
155+
page.getByTestId('search-attribute-select-button'),
156+
).toBeVisible();
157+
158+
await page
159+
.getByTestId('search-attribute-close-button')
160+
.and(page.locator(':visible'))
161+
.click();
162+
await expect(
163+
page.getByTestId('search-attribute-select-button'),
164+
).toBeHidden();
165+
});
166+
});
58167
});

0 commit comments

Comments
 (0)