Skip to content

Commit a734510

Browse files
Tomer9000claude
authored andcommitted
[JENKINS-73556] Disable Build while reactive parameters are updating
Users could submit the form while cascade/dynamic reference parameters were still recalculating, building with empty or stale values (GH-868). A LoadingGuard counts pending updates: every update is scheduled through scheduleUpdate(), and proxy calls count until their callback has rendered. While anything is pending, the form's submit buttons are disabled and submit is blocked; a failed update keeps them blocked with a reload message. The proxy now also returns the response promise, so awaiting a call waits for its callback (JENKINS-75793, GH-902, same fix as #960). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent ca8a1e7 commit a734510

7 files changed

Lines changed: 481 additions & 21 deletions

File tree

‎CHANGES.md‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Active Choices plug-in changelog
22

3-
## Version 2.8.11 (????/??/??)
3+
## Version 2.8.11 (2026/10/12)
44

55
- Bump @humanfs/node from 0.16.6 to 0.16.8
66
- Bump eslint from 10.9.1 to 10.11.0
@@ -16,6 +16,10 @@
1616
- Bump webpack from 5.109.2 to 5.111.1
1717
- Bump webpack-cli from 7.2.2 to 7.2.3
1818
- Prevent infinite loop when a Cascade Reference Parameter references itself #1017 (thanks @B-Sp67)
19+
- JENKINS-73556, GH-868: Disable the Build button while reactive parameters are updating,
20+
and keep it disabled with a reload message if an update fails #868 (thanks @Tomer9000)
21+
- JENKINS-75793, GH-902: Wait for a parameter to be rendered before updating the parameters
22+
that reference it #902 (thanks @Tomer9000)
1923

2024
## Version 2.8.10 (2026/08/25)
2125

‎package.json‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@
6868
],
6969
"testMatch": [
7070
"<rootDir>/src/test/js/*.test.ts"
71-
]
71+
],
72+
"moduleFileExtensions": ["js", "mjs", "cjs", "jsx", "ts", "tsx", "json", "node", "es6"],
73+
"transform": {
74+
"\\.(es6|[jt]sx?)$": "babel-jest"
75+
}
7276
}
7377
}

‎src/main/js/LoadingGuard.ts‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
// Keeps the submit buttons of a parameters form disabled while Active Choices parameters are updating (JENKINS-73556).
2+
export default class LoadingGuard {
3+
4+
doc: Document;
5+
pending: number = 0;
6+
failed: boolean = false;
7+
busySince: number = 0;
8+
ticker: number | null = null;
9+
forms: HTMLFormElement[] = [];
10+
statuses: HTMLElement[] = [];
11+
// Only buttons this guard disabled are re-enabled, so other scripts' disabled state is kept.
12+
disabledByGuard: HTMLButtonElement[] = [];
13+
14+
constructor(doc: Document) {
15+
this.doc = doc;
16+
// Blocks implicit submission (Enter in a text field) and programmatic requestSubmit() while busy.
17+
doc.addEventListener('submit', (e: Event) => {
18+
if (this.isBusy() && this.forms.indexOf(e.target as HTMLFormElement) >= 0) {
19+
e.preventDefault();
20+
e.stopImmediatePropagation();
21+
}
22+
}, true);
23+
}
24+
25+
/** Marks an update as started; the returned function marks it finished and is safe to call more than once. */
26+
public begin(element?: Element): () => void {
27+
this.watch(element);
28+
if (this.pending === 0) {
29+
this.busySince = Date.now();
30+
}
31+
this.pending++;
32+
this.render();
33+
let finished = false;
34+
return () => {
35+
if (finished) {
36+
return;
37+
}
38+
finished = true;
39+
this.pending--;
40+
this.render();
41+
};
42+
}
43+
44+
/** A failed update leaves a stale value behind, so the form stays blocked until the page is reloaded. */
45+
public fail(): void {
46+
this.failed = true;
47+
this.render();
48+
}
49+
50+
public isBusy(): boolean {
51+
return this.failed || this.pending > 0;
52+
}
53+
54+
private watch(element?: Element): void {
55+
let form = element ? element.closest('form') : null;
56+
if (!form || this.forms.indexOf(form) >= 0) {
57+
return;
58+
}
59+
this.forms.push(form);
60+
let buttons = this.submitButtons(form);
61+
let bar = buttons.length > 0 ? buttons[buttons.length - 1].parentElement : null;
62+
if (bar) {
63+
let status = this.doc.createElement('span');
64+
status.className = 'ac-loading-status';
65+
status.setAttribute('aria-live', 'polite');
66+
bar.appendChild(status);
67+
this.statuses.push(status);
68+
}
69+
}
70+
71+
// Submit controls of the form itself; anything inside a parameter (div[name=parameter]) is left alone.
72+
private submitButtons(form: HTMLFormElement): HTMLButtonElement[] {
73+
let buttons: HTMLButtonElement[] = [];
74+
let candidates = form.querySelectorAll('button, input[type="submit"]');
75+
for (let i = 0; i < candidates.length; i++) {
76+
let button = candidates[i] as HTMLButtonElement;
77+
if (button.type === 'submit' && !button.closest('[name="parameter"]')) {
78+
buttons.push(button);
79+
}
80+
}
81+
return buttons;
82+
}
83+
84+
private render(): void {
85+
let busy = this.isBusy();
86+
if (busy) {
87+
for (let i = 0; i < this.forms.length; i++) {
88+
let buttons = this.submitButtons(this.forms[i]);
89+
for (let j = 0; j < buttons.length; j++) {
90+
if (!buttons[j].disabled) {
91+
buttons[j].disabled = true;
92+
this.disabledByGuard.push(buttons[j]);
93+
}
94+
}
95+
}
96+
} else {
97+
for (let i = 0; i < this.disabledByGuard.length; i++) {
98+
this.disabledByGuard[i].disabled = false;
99+
}
100+
this.disabledByGuard = [];
101+
}
102+
let text = '';
103+
if (this.failed) {
104+
text = 'A parameter failed to load. Reload the page before building.';
105+
} else if (busy) {
106+
text = `Calculating parameters… ${Math.round((Date.now() - this.busySince) / 1000)}s`;
107+
}
108+
for (let i = 0; i < this.statuses.length; i++) {
109+
this.statuses[i].textContent = text;
110+
this.statuses[i].classList.toggle('ac-loading-status--error', this.failed);
111+
}
112+
// Ticks the elapsed seconds while busy; stops once settled or failed.
113+
if (busy && !this.failed && this.ticker === null && this.statuses.length > 0) {
114+
this.ticker = window.setInterval(() => this.render(), 1000);
115+
} else if ((!busy || this.failed) && this.ticker !== null) {
116+
window.clearInterval(this.ticker);
117+
this.ticker = null;
118+
}
119+
}
120+
}

‎src/main/js/UnoChoice.es6‎

Lines changed: 56 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
* THE SOFTWARE.
2323
*/
2424
import Util from './Util.ts';
25+
import LoadingGuard from './LoadingGuard.ts';
2526

2627
/*
2728
* At the time of writing, requestIdleCallback is still not available in Safari,
@@ -82,6 +83,8 @@ jQuery3.noConflict();
8283
*/
8384
var UnoChoice = UnoChoice || (jQuery3 => {
8485
let util = new Util(jQuery3);
86+
// Keeps Build disabled while any parameter is updating (JENKINS-73556).
87+
let loadingGuard = new LoadingGuard(document);
8588
// The final public object
8689
let instance = {};
8790
let SEPARATOR = '__LESEP__';
@@ -102,6 +105,7 @@ var UnoChoice = UnoChoice || (jQuery3 => {
102105
this.proxy = proxy;
103106
this.referencedParameters = [];
104107
this.filterElement = null;
108+
trackProxy(proxy);
105109
}
106110
/**
107111
* Gets the parameter name.
@@ -370,9 +374,7 @@ var UnoChoice = UnoChoice || (jQuery3 => {
370374
let other = cascadeParameters[i];
371375
if (this.referencesMe(other)) {
372376
console.log(`[${this.paramName}] - CascadeParameter#update - Updating ${other.getParameterName()} from ${this.getParameterName()}`);
373-
window.requestIdleCallback(async () => {
374-
await other.update(other.getParameterName() === this.getParameterName());
375-
})
377+
scheduleUpdate(other, other.getParameterName() === this.getParameterName());
376378
}
377379
}
378380
}
@@ -435,10 +437,9 @@ var UnoChoice = UnoChoice || (jQuery3 => {
435437
//_self.cascadeParameter.loading(true);
436438
jQuery3(".behavior-loading").show();
437439
// start updating in separate async function so browser will be able to repaint and show 'loading' animation , see JENKINS-34487
438-
window.requestIdleCallback(async () => {
439-
await _self.cascadeParameter.update(false);
440+
scheduleUpdate(_self.cascadeParameter, false).then(() => {
440441
jQuery3(".behavior-loading").hide();
441-
})
442+
});
442443
}
443444
});
444445
cascadeParameter.getReferencedParameters().push(this);
@@ -465,6 +466,7 @@ var UnoChoice = UnoChoice || (jQuery3 => {
465466
this.paramElement = paramElement;
466467
this.proxy = proxy;
467468
this.referencedParameters = [];
469+
trackProxy(proxy);
468470
}
469471
/**
470472
* Extend the cascade parameter.
@@ -556,9 +558,7 @@ var UnoChoice = UnoChoice || (jQuery3 => {
556558
let other = cascadeParameters[i];
557559
if (this.referencesMe(other)) {
558560
console.log(`[${this.paramName}] - DynamicReferenceParameter#update - Updating ${other.paramName} from ${this.paramName}`);
559-
window.requestIdleCallback(async () => {
560-
await other.update(true);
561-
})
561+
scheduleUpdate(other, true);
562562
}
563563
}
564564
}
@@ -847,6 +847,31 @@ var UnoChoice = UnoChoice || (jQuery3 => {
847847
return value;
848848
}
849849

850+
// Updates the parameter once the browser is idle; Build stays disabled from now until the returned promise settles.
851+
function scheduleUpdate(parameter, avoidRecursion) {
852+
let done = loadingGuard.begin(parameter.getParameterElement());
853+
return new Promise(resolve => {
854+
window.requestIdleCallback(async () => {
855+
try {
856+
await parameter.update(avoidRecursion);
857+
} catch (e) {
858+
loadingGuard.fail();
859+
console.error(`[${parameter.getParameterName()}] - scheduleUpdate - Update failed`, e);
860+
} finally {
861+
done();
862+
resolve();
863+
}
864+
});
865+
});
866+
}
867+
868+
// Calls made through this proxy keep Build disabled until their response has been rendered.
869+
function trackProxy(proxy) {
870+
if (proxy) {
871+
proxy.loadingGuard = loadingGuard;
872+
}
873+
}
874+
850875
// Hacks in Jenkins core
851876
/**
852877
* <p>This function is the same as makeStaplerProxy available in Jenkins core, but executes calls
@@ -901,6 +926,14 @@ var UnoChoice = UnoChoice || (jQuery3 => {
901926
// what is rendered when, and properly chain certain actions (e.g.
902927
// a dependant parameter is only rendered after its parent/referenced
903928
// parameter).
929+
// JENKINS-73556: the call counts as pending until its callback has rendered the response.
930+
let guard = proxy.loadingGuard;
931+
let done = guard ? guard.begin() : () => {};
932+
let failed = error => {
933+
if (guard) guard.fail();
934+
done();
935+
console.error(`makeStaplerProxy2 - ${methodName} failed`, error);
936+
};
904937
await fetch(url + methodName, {
905938
method: 'POST',
906939
headers: headers,
@@ -912,25 +945,32 @@ var UnoChoice = UnoChoice || (jQuery3 => {
912945
status: response.status,
913946
statusText: response.statusText,
914947
};
948+
// Returned, so that awaiting this call also waits for its callback (JENKINS-75793).
915949
if (response.headers.has('content-type') && response.headers.get('content-type').startsWith('application/json')) {
916-
response.json().then(function (responseObject) {
950+
return response.json().then(function (responseObject) {
917951
t.responseObject = function () {
918952
return responseObject;
919953
};
920954
t.responseJSON = responseObject;
921955
if (callback != null) {
922956
callback(t);
923957
}
924-
});
958+
}).then(done, failed);
925959
} else {
926-
response.text().then(function (responseText) {
960+
return response.text().then(function (responseText) {
927961
t.responseText = responseText;
928962
if (callback != null) {
929963
callback(t);
930964
}
931-
});
965+
}).then(done, failed);
932966
}
967+
} else {
968+
// The parameter keeps its previous value, so it must not be submitted.
969+
failed(`HTTP ${response.status} ${response.statusText}`);
933970
}
971+
}, function (error) {
972+
failed(error);
973+
throw error;
934974
})
935975
}
936976
};
@@ -1019,9 +1059,7 @@ var UnoChoice = UnoChoice || (jQuery3 => {
10191059

10201060
// call update methods in Java passing the HTML values
10211061
console.log(`[${name}] - renderCascadeChoiceParameter - Updating cascade of parameter [${name}] ...`);
1022-
window.requestIdleCallback(async () => {
1023-
await cascadeParameter.update(false);
1024-
})
1062+
scheduleUpdate(cascadeParameter, false);
10251063
} else {
10261064
console.log(`[${name}] - renderCascadeChoiceParameter - Parameter error: Missing parameter [${paramName}] HTML element!`);
10271065
}
@@ -1104,9 +1142,7 @@ var UnoChoice = UnoChoice || (jQuery3 => {
11041142

11051143
// call update methods in Java passing the HTML values
11061144
console.log(`[${name}] - renderDynamicRenderParameter - Updating cascade of parameter [${name}] ...`);
1107-
window.requestIdleCallback(async () => {
1108-
await dynamicParameter.update(false);
1109-
})
1145+
scheduleUpdate(dynamicParameter, false);
11101146
} else {
11111147
console.log(`[${name}] - renderDynamicRenderParameter - Parameter error: Missing parameter [${paramName}] HTML element!`);
11121148
}
@@ -1120,6 +1156,7 @@ var UnoChoice = UnoChoice || (jQuery3 => {
11201156
instance.ReferencedParameter = ReferencedParameter;
11211157
instance.FilterElement = FilterElement;
11221158
instance.makeStaplerProxy2 = makeStaplerProxy2;
1159+
instance.loadingGuard = loadingGuard;
11231160
instance.cascadeParameters = cascadeParameters;
11241161
instance.renderChoiceParameter = renderChoiceParameter;
11251162
instance.renderCascadeChoiceParameter = renderCascadeChoiceParameter;

‎src/main/webapp/css/unochoice.css‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,3 +59,14 @@
5959
box-shadow: var(--form-input-glow--focus);
6060
outline: none;
6161
}
62+
63+
/* JENKINS-73556: shown next to Build while parameters are updating */
64+
.ac-loading-status {
65+
align-self: center;
66+
margin-left: 0.75rem;
67+
color: var(--text-color-secondary);
68+
}
69+
70+
.ac-loading-status--error {
71+
color: var(--error-color);
72+
}

0 commit comments

Comments
 (0)