Skip to content

Commit a08d2d2

Browse files
yiannisnikolopouloscursoragentkibanamachinejasonrhodes
authored
[Alerting v2] Fix YAML blur handler + splitQuery round-trip tests (#269779)
## Summary Scoped down from the original PR after #269584 merged. The split-mode sync and YAML toggle-off dispatch fixes are now redundant — this PR retains only the changes that #269584 did not address. ### Changes 1. **`YamlRuleForm` blur handler fix:** In the compose-discover flyout the RHF form uses `ComposeFormValues` (with `query: RuleQuery`), but `YamlRuleForm`'s blur handler called `reset(FormValues)` directly — overwriting the form with the wrong shape (`evaluation.query.base` instead of `RuleQuery`). After #269584 this no longer clears the sandbox visually (the sandbox now reads from the draft, not RHF), but it silently corrupts the form state, which could cause malformed API payloads at submit time. Adds an `onBlurSync` prop so the flyout can route through `formValuesFromYamlToCompose`, cancelling the pending debounce and syncing the draft in one shot. 2. **`YamlParseResult` discriminated union:** Changes the type from two independently-nullable fields to a proper discriminated union (`{ values: FormValues; error: null } | { values: null; error: string }`) so TypeScript narrows automatically after error checks, eliminating non-null assertions. (Addresses Jason's [review feedback](#269779 (review)).) 3. **`splitQuery` round-trip idempotency tests:** Asserts that `splitQuery(join(split(q))) === split(q)`, which is the invariant the split-mode sync relies on. Closes [#269777](#270168) ## How to test 1. Navigate to **Alerting v2 → Rules list** → Create a rule via the flyout 2. Open the Sandbox, write a query, click **Apply changes** 3. Toggle to **YAML view** 4. Edit the query in YAML (e.g. change a threshold value) 5. Click outside the YAML editor (blur) 6. Toggle back to **Form view** and click **Create rule** 7. **Verify:** The API request payload (`evaluation.query.base`) contains the edited query value — the YAML edit persisted correctly through the blur → RHF → submit pipeline --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Jason Rhodes <jason.rhodes@elastic.co>
1 parent 120c2cf commit a08d2d2

4 files changed

Lines changed: 58 additions & 22 deletions

File tree

x-pack/platform/packages/shared/response-ops/alerting-v2-rule-form/flyout/compose_discover/compose_discover_flyout.tsx

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,15 @@ export function ComposeDiscoverFlyout<TWorkflow extends object = object>({
419419
[runYamlParse]
420420
);
421421

422+
const handleBlurSync = useCallback(
423+
(values: FormValues) => {
424+
cancelYamlParse();
425+
methods.reset(formValuesFromYamlToCompose(values));
426+
syncSandbox();
427+
},
428+
[cancelYamlParse, methods, syncSandbox]
429+
);
430+
422431
const handleToggleYamlMode = useCallback(
423432
(enabled: boolean) => {
424433
if (enabled) {
@@ -480,7 +489,7 @@ export function ComposeDiscoverFlyout<TWorkflow extends object = object>({
480489
const result = parseYamlToFormValues(yamlText);
481490
if (result.values) {
482491
methods.reset(formValuesFromYamlToCompose(result.values));
483-
// No syncForm() here: draft is temporarily stale after methods.reset(), but
492+
// No syncSandbox() here: sandbox is temporarily stale after methods.reset(), but
484493
// we're about to submit. On success the flyout closes; on failure the user is still
485494
// in YAML mode and handleToggleYamlMode(false) will resync draft when they switch back.
486495
}
@@ -576,6 +585,7 @@ export function ComposeDiscoverFlyout<TWorkflow extends object = object>({
576585
services={baseServices}
577586
yamlText={yamlText}
578587
setYamlText={handleSetYamlText}
588+
onBlurSync={handleBlurSync}
579589
isSubmitting={isSaving}
580590
/>
581591
</React.Suspense>

x-pack/platform/packages/shared/response-ops/alerting-v2-rule-form/flyout/compose_discover/use_heuristic_split.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,34 @@ describe('splitQuery', () => {
148148
});
149149
});
150150

151+
describe('round-trip idempotency (join + re-split)', () => {
152+
it('round-trips a STATS + WHERE query through join and re-split', () => {
153+
const { base, alertBlock } = splitQuery('FROM logs-* | STATS c = COUNT(*) | WHERE c > 5');
154+
const reassembled = [base, alertBlock].filter(Boolean).join('\n');
155+
const resplit = splitQuery(reassembled);
156+
expect(resplit.base).toBe(base);
157+
expect(resplit.alertBlock).toBe(alertBlock);
158+
});
159+
160+
it('round-trips a multi-pipe query', () => {
161+
const { base, alertBlock } = splitQuery(
162+
'FROM logs-* | EVAL x = 1 | STATS c = COUNT(*) BY host | WHERE c > 10 | SORT c DESC'
163+
);
164+
const reassembled = [base, alertBlock].filter(Boolean).join('\n');
165+
const resplit = splitQuery(reassembled);
166+
expect(resplit.base).toBe(base);
167+
expect(resplit.alertBlock).toBe(alertBlock);
168+
});
169+
170+
it('round-trips a WHERE-only (no STATS) query', () => {
171+
const { base, alertBlock } = splitQuery('FROM logs-* | EVAL x = 1 | WHERE x > 0');
172+
const reassembled = [base, alertBlock].filter(Boolean).join('\n');
173+
const resplit = splitQuery(reassembled);
174+
expect(resplit.base).toBe(base);
175+
expect(resplit.alertBlock).toBe(alertBlock);
176+
});
177+
});
178+
151179
describe('reason field', () => {
152180
it('reason is no_stats when neither STATS nor WHERE found', () => {
153181
expect(splitQuery('FROM logs-*').reason).toBe('no_stats');

x-pack/platform/packages/shared/response-ops/alerting-v2-rule-form/form/utils/yaml_form_utils.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,7 @@ import {
1313
deriveRecoveryDelayModeFromStateTransition,
1414
} from './rule_request_mappers';
1515

16-
export interface YamlParseResult {
17-
values: FormValues | null;
18-
error: string | null;
19-
}
16+
export type YamlParseResult = { values: FormValues; error: null } | { values: null; error: string };
2017

2118
const parseArtifacts = (artifacts: unknown): FormValues['artifacts'] => {
2219
if (!Array.isArray(artifacts)) return undefined;

x-pack/platform/packages/shared/response-ops/alerting-v2-rule-form/form/yaml_rule_form.tsx

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,13 @@ export interface YamlRuleFormProps {
3636
yamlText: string;
3737
/** Setter for the lifted YAML buffer. */
3838
setYamlText: (yaml: string) => void;
39+
/**
40+
* Synchronous commit for blur events. In the compose-discover flyout the RHF
41+
* form uses `ComposeFormValues`, so a raw `reset(FormValues)` would corrupt it.
42+
* The flyout passes a callback that routes through `formValuesFromYamlToCompose`.
43+
* When absent (standalone context), blur falls back to `useFormContext().reset()`.
44+
*/
45+
onBlurSync?: (values: FormValues) => void;
3946
}
4047

4148
/**
@@ -60,6 +67,7 @@ export const YamlRuleForm = ({
6067
isSubmitting = false,
6168
yamlText,
6269
setYamlText,
70+
onBlurSync,
6371
}: YamlRuleFormProps) => {
6472
const [error, setError] = useState<string | null>(null);
6573
const { reset } = useFormContext<FormValues>();
@@ -70,40 +78,33 @@ export const YamlRuleForm = ({
7078
search: services.data.search.search,
7179
});
7280

73-
const applyYamlValuesToForm = useCallback(
74-
(values: FormValues) => {
75-
reset(values);
76-
},
77-
[reset]
78-
);
79-
8081
const handleSubmit = useCallback(
8182
(e: React.FormEvent) => {
8283
e.preventDefault();
8384
const result = parseYamlToFormValues(yamlText);
84-
if (result.error) {
85+
if (result.error !== null) {
8586
setError(result.error);
8687
return;
8788
}
88-
if (result.values) {
89-
setError(null);
90-
onSubmit?.(result.values);
91-
}
89+
setError(null);
90+
onSubmit?.(result.values);
9291
},
9392
[yamlText, onSubmit]
9493
);
9594

9695
const handleBlur = useCallback(() => {
9796
const result = parseYamlToFormValues(yamlText);
98-
if (result.error) {
97+
if (result.error !== null) {
9998
setError(result.error);
10099
return;
101100
}
102-
if (result.values) {
103-
setError(null);
104-
applyYamlValuesToForm(result.values);
101+
setError(null);
102+
if (onBlurSync) {
103+
onBlurSync(result.values);
104+
} else {
105+
reset(result.values);
105106
}
106-
}, [yamlText, applyYamlValuesToForm]);
107+
}, [yamlText, onBlurSync, reset]);
107108

108109
const handleYamlChange = useCallback(
109110
(newYaml: string) => {

0 commit comments

Comments
 (0)