Skip to content

Commit 9ee3ed8

Browse files
[8.18] [ML] [Anomaly Detection] Fixes job ID already exists validation in wizard (elastic#236462) (elastic#237077)
# Backport This will backport the following commits from `main` to `8.18`: - [[ML] [Anomaly Detection] Fixes job ID already exists validation in wizard (elastic#236462)](elastic#236462) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Konrad Krasocki","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-10-01T09:17:47Z","message":"[ML] [Anomaly Detection] Fixes job ID already exists validation in wizard (elastic#236462)\n\n## Summary\n\nFixes elastic#214997\n\nThis PR:\n- Fixes the Job ID validation by using immutable snapshots of `{ jobId,\nmlApi }` so `distinctUntilChanged` detects actual `jobId` changes\ninstead of comparing the same mutated object and suppressing emissions.\n[[1]](https://github.com/elastic/kibana/pull/236462/files#diff-13b5c7aaf21fc09b1151ed023dd98ac479b47ef567e61bf989b523d9f31763d5R116-R117)\n- Adds unit tests to `jobIdValidator` for common scenarios\n\n\n\nhttps://github.com/user-attachments/assets/b56aabd4-b1b0-4455-b677-d56968b52e7f\n\n\n\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [ ] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [ ] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [ ] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [ ] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n- [ ] Review the [backport\nguidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)\nand apply applicable `backport:*` labels.\n\n### Identify risks\n\nDoes this PR introduce any risks? For example, consider risks like hard\nto test bugs, performance regression, potential of data loss.\n\nDescribe the risk, its severity, and mitigation for each identified\nrisk. Invite stakeholders and evaluate how to proceed before merging.\n\n- [ ] [See some risk\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\n- [ ] ...","sha":"6e35845d5b0c04317a42e8de590142ffa98b0183","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug",":ml","release_note:skip","Team:ML","backport:all-open","v9.2.0"],"title":"[ML] [Anomaly Detection] Fixes job ID already exists validation in wizard","number":236462,"url":"https://github.com/elastic/kibana/pull/236462","mergeCommit":{"message":"[ML] [Anomaly Detection] Fixes job ID already exists validation in wizard (elastic#236462)\n\n## Summary\n\nFixes elastic#214997\n\nThis PR:\n- Fixes the Job ID validation by using immutable snapshots of `{ jobId,\nmlApi }` so `distinctUntilChanged` detects actual `jobId` changes\ninstead of comparing the same mutated object and suppressing emissions.\n[[1]](https://github.com/elastic/kibana/pull/236462/files#diff-13b5c7aaf21fc09b1151ed023dd98ac479b47ef567e61bf989b523d9f31763d5R116-R117)\n- Adds unit tests to `jobIdValidator` for common scenarios\n\n\n\nhttps://github.com/user-attachments/assets/b56aabd4-b1b0-4455-b677-d56968b52e7f\n\n\n\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [ ] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [ ] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [ ] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [ ] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n- [ ] Review the [backport\nguidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)\nand apply applicable `backport:*` labels.\n\n### Identify risks\n\nDoes this PR introduce any risks? For example, consider risks like hard\nto test bugs, performance regression, potential of data loss.\n\nDescribe the risk, its severity, and mitigation for each identified\nrisk. Invite stakeholders and evaluate how to proceed before merging.\n\n- [ ] [See some risk\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\n- [ ] ...","sha":"6e35845d5b0c04317a42e8de590142ffa98b0183"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/236462","number":236462,"mergeCommit":{"message":"[ML] [Anomaly Detection] Fixes job ID already exists validation in wizard (elastic#236462)\n\n## Summary\n\nFixes elastic#214997\n\nThis PR:\n- Fixes the Job ID validation by using immutable snapshots of `{ jobId,\nmlApi }` so `distinctUntilChanged` detects actual `jobId` changes\ninstead of comparing the same mutated object and suppressing emissions.\n[[1]](https://github.com/elastic/kibana/pull/236462/files#diff-13b5c7aaf21fc09b1151ed023dd98ac479b47ef567e61bf989b523d9f31763d5R116-R117)\n- Adds unit tests to `jobIdValidator` for common scenarios\n\n\n\nhttps://github.com/user-attachments/assets/b56aabd4-b1b0-4455-b677-d56968b52e7f\n\n\n\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [ ] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [ ] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [ ] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [ ] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n- [ ] Review the [backport\nguidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing)\nand apply applicable `backport:*` labels.\n\n### Identify risks\n\nDoes this PR introduce any risks? For example, consider risks like hard\nto test bugs, performance regression, potential of data loss.\n\nDescribe the risk, its severity, and mitigation for each identified\nrisk. Invite stakeholders and evaluate how to proceed before merging.\n\n- [ ] [See some risk\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\n- [ ] ...","sha":"6e35845d5b0c04317a42e8de590142ffa98b0183"}}]}] BACKPORT--> Co-authored-by: Konrad Krasocki <[email protected]>
1 parent 3f0995a commit 9ee3ed8

File tree

2 files changed

+76
-0
lines changed

2 files changed

+76
-0
lines changed
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
import { Subject, of } from 'rxjs';
9+
import { jobIdValidator } from './validators';
10+
11+
describe('jobIdValidator', () => {
12+
const makeMlApi = (responses: Record<string, boolean>) => ({
13+
jobs: {
14+
jobsExist$: (ids: string[]) =>
15+
of(
16+
ids.reduce((acc, id) => {
17+
acc[id] = { exists: responses[id] ?? false };
18+
return acc;
19+
}, {} as any)
20+
),
21+
},
22+
});
23+
24+
const makeJobCreator = (jobId: string, mlApi: any) => ({ jobId, mlApi } as any);
25+
26+
it('emits valid=true when id does not exist', (done) => {
27+
const mlApi = makeMlApi({}); // no existing ids
28+
const source$ = new Subject<any>();
29+
jobIdValidator(source$).subscribe((v) => {
30+
expect(v?.jobIdExists.valid).toBe(true);
31+
done();
32+
});
33+
source$.next(makeJobCreator('new_id', mlApi));
34+
});
35+
36+
it('emits valid=false when id exists', (done) => {
37+
const mlApi = makeMlApi({ taken_id: true });
38+
const source$ = new Subject<any>();
39+
jobIdValidator(source$).subscribe((v) => {
40+
expect(v?.jobIdExists.valid).toBe(false);
41+
expect(v?.jobIdExists.message).toBeDefined();
42+
done();
43+
});
44+
source$.next(makeJobCreator('taken_id', mlApi));
45+
});
46+
47+
it('suppresses emission when jobId unchanged', () => {
48+
const mlApi = makeMlApi({});
49+
const source$ = new Subject<any>();
50+
const emissions: any[] = [];
51+
jobIdValidator(source$).subscribe((v) => emissions.push(v));
52+
53+
const jc = makeJobCreator('same_id', mlApi);
54+
source$.next(jc);
55+
source$.next(jc); // same ref, same id
56+
source$.next(makeJobCreator('same_id', mlApi)); // new ref, same id
57+
58+
expect(emissions.length).toBe(1);
59+
});
60+
61+
it('emits again when jobId changes', () => {
62+
const mlApi = makeMlApi({ taken_id: true });
63+
const source$ = new Subject<any>();
64+
const emissions: any[] = [];
65+
jobIdValidator(source$).subscribe((v) => emissions.push(v));
66+
67+
source$.next(makeJobCreator('free_id', mlApi)); // valid
68+
source$.next(makeJobCreator('taken_id', mlApi)); // invalid
69+
70+
expect(emissions).toHaveLength(2);
71+
expect(emissions[0].jobIdExists.valid).toBe(true);
72+
expect(emissions[1].jobIdExists.valid).toBe(false);
73+
});
74+
});

x-pack/platform/plugins/shared/ml/public/application/jobs/new_job/common/job_validator/validators.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,8 @@ export function cardinalityValidator(
113113

114114
export function jobIdValidator(jobCreator$: Subject<JobCreator>): Observable<JobExistsResult> {
115115
return jobCreator$.pipe(
116+
// Emit a fresh { jobId, mlApi } snapshot to catch jobId changes
117+
map((jc) => ({ jobId: jc.jobId, mlApi: jc.mlApi })),
116118
// No need to perform an API call if the analysis configuration hasn't been changed
117119
distinctUntilChanged(
118120
(prevJobCreator, currJobCreator) => prevJobCreator.jobId === currJobCreator.jobId

0 commit comments

Comments
 (0)