* [pve-devel] [PATCH manager 1/1] fix: ha: set ha-managed param default and unchecked value to undefined
@ 2025-11-17 13:18 Michael Köppl
2025-11-17 16:47 ` Thomas Lamprecht
0 siblings, 1 reply; 3+ messages in thread
From: Michael Köppl @ 2025-11-17 13:18 UTC (permalink / raw)
To: pve-devel
This avoids sending the parameter with the default value when it's not
necessary, since the API already defines the param as optional and
assigns a default value of 0.
Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
Thanks to @Friedrich for bringing this to my attention. A case where
this occurs is in clusters where at least one of the nodes does not yet
support the ha-managed param and a VM is created on that node from the
web UI of a node that already supports it. Although in general it can
probably be expected that all nodes should be upgraded to a new version,
this avoids failing the CreateWizard dialog entirely for cases where the
"Add to HA" checkbox wasn't even checked at basically no cost because
the API already defines a default value of 0 and considers the param
optional.
www/manager6/lxc/CreateWizard.js | 4 ++--
www/manager6/qemu/CreateWizard.js | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/www/manager6/lxc/CreateWizard.js b/www/manager6/lxc/CreateWizard.js
index f35709f14..024536c58 100644
--- a/www/manager6/lxc/CreateWizard.js
+++ b/www/manager6/lxc/CreateWizard.js
@@ -85,8 +85,8 @@ Ext.define('PVE.lxc.CreateWizard', {
{
xtype: 'proxmoxcheckbox',
name: 'ha-managed',
- uncheckedValue: 0,
- defaultValue: 0,
+ uncheckedValue: undefined,
+ defaultValue: undefined,
fieldLabel: gettext('Add to HA'),
},
],
diff --git a/www/manager6/qemu/CreateWizard.js b/www/manager6/qemu/CreateWizard.js
index e0c56bc0b..564ca1176 100644
--- a/www/manager6/qemu/CreateWizard.js
+++ b/www/manager6/qemu/CreateWizard.js
@@ -99,8 +99,8 @@ Ext.define('PVE.qemu.CreateWizard', {
{
xtype: 'proxmoxcheckbox',
name: 'ha-managed',
- uncheckedValue: 0,
- defaultValue: 0,
+ uncheckedValue: undefined,
+ defaultValue: undefined,
fieldLabel: gettext('Add to HA'),
},
],
--
2.47.3
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [pve-devel] [PATCH manager 1/1] fix: ha: set ha-managed param default and unchecked value to undefined
2025-11-17 13:18 [pve-devel] [PATCH manager 1/1] fix: ha: set ha-managed param default and unchecked value to undefined Michael Köppl
@ 2025-11-17 16:47 ` Thomas Lamprecht
2025-11-17 17:35 ` Michael Köppl
0 siblings, 1 reply; 3+ messages in thread
From: Thomas Lamprecht @ 2025-11-17 16:47 UTC (permalink / raw)
To: Proxmox VE development discussion, Michael Köppl
The commit subject could be a bit improved, as of now we do not use plain
"fix" commits, besides that it affects the ui subsystem, which is a major
one in this repo, so naming it makes sense, then it's not really for ha
itself but rather for the (guest) create wizard, and finally, the commit
message and it's subject should focus on what's fixed and ideally (in short)
why thefix works and was chosen rather than describing how it is done,
especially not if that's just then a rather literal description of the
actual diff itself.
E.g. here the following would be a lot clearer to me:
"ui: create wizard: only submit ha-managed if checked for better compat"
Am 17.11.25 um 14:17 schrieb Michael Köppl:
> This avoids sending the parameter with the default value when it's not
> necessary, since the API already defines the param as optional and
> assigns a default value of 0.
Why does this work though, 0 and undefined seem close enough for anybody
without in-depth knowledge about ExtJS and/or our widget-toolkit.
Would be nice to reference upstream docs for chechbox's uncheckedValue
config parameter, which plays the core part here:
> By default this is undefined, which results in nothing being submitted
> for the checkbox field when the form is submitted
-- https://docs.sencha.com/extjs/7.0.0/classic/Ext.form.field.Checkbox.html#cfg-uncheckedValue
>
> Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
> ---
> Thanks to @Friedrich for bringing this to my attention. A case where
> this occurs is in clusters where at least one of the nodes does not yet
> support the ha-managed param and a VM is created on that node from the
> web UI of a node that already supports it. Although in general it can
> probably be expected that all nodes should be upgraded to a new version,
> this avoids failing the CreateWizard dialog entirely for cases where the
> "Add to HA" checkbox wasn't even checked at basically no cost because
> the API already defines a default value of 0 and considers the param
> optional.
>
> www/manager6/lxc/CreateWizard.js | 4 ++--
> www/manager6/qemu/CreateWizard.js | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/www/manager6/lxc/CreateWizard.js b/www/manager6/lxc/CreateWizard.js
> index f35709f14..024536c58 100644
> --- a/www/manager6/lxc/CreateWizard.js
> +++ b/www/manager6/lxc/CreateWizard.js
> @@ -85,8 +85,8 @@ Ext.define('PVE.lxc.CreateWizard', {
> {
> xtype: 'proxmoxcheckbox',
> name: 'ha-managed',
> - uncheckedValue: 0,
> - defaultValue: 0,
> + uncheckedValue: undefined,
> + defaultValue: undefined,
Both are the default for (proxmox)checkbox, so could be just omitted? Or if, then
I'd just set "uncheckedValue" explicitly and add a small comment there so that it's
a bit more explicit and telling.
> fieldLabel: gettext('Add to HA'),
> },
> ],
> diff --git a/www/manager6/qemu/CreateWizard.js b/www/manager6/qemu/CreateWizard.js
> index e0c56bc0b..564ca1176 100644
> --- a/www/manager6/qemu/CreateWizard.js
> +++ b/www/manager6/qemu/CreateWizard.js
> @@ -99,8 +99,8 @@ Ext.define('PVE.qemu.CreateWizard', {
> {
> xtype: 'proxmoxcheckbox',
> name: 'ha-managed',
> - uncheckedValue: 0,
> - defaultValue: 0,
> + uncheckedValue: undefined,
> + defaultValue: undefined,
Same here.
> fieldLabel: gettext('Add to HA'),
> },
> ],
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [pve-devel] [PATCH manager 1/1] fix: ha: set ha-managed param default and unchecked value to undefined
2025-11-17 16:47 ` Thomas Lamprecht
@ 2025-11-17 17:35 ` Michael Köppl
0 siblings, 0 replies; 3+ messages in thread
From: Michael Köppl @ 2025-11-17 17:35 UTC (permalink / raw)
To: Thomas Lamprecht, Proxmox VE development discussion, Michael Köppl
Thanks for the detailed feedback! Sent a v2 with a better commit
message and only uncheckedValue set explicitly.
https://lore.proxmox.com/pve-devel/20251117173321.240894-1-m.koeppl@proxmox.com
On Mon Nov 17, 2025 at 5:47 PM CET, Thomas Lamprecht wrote:
> The commit subject could be a bit improved, as of now we do not use plain
> "fix" commits, besides that it affects the ui subsystem, which is a major
> one in this repo, so naming it makes sense, then it's not really for ha
> itself but rather for the (guest) create wizard, and finally, the commit
> message and it's subject should focus on what's fixed and ideally (in short)
> why thefix works and was chosen rather than describing how it is done,
> especially not if that's just then a rather literal description of the
> actual diff itself.
>
> E.g. here the following would be a lot clearer to me:
>
> "ui: create wizard: only submit ha-managed if checked for better compat"
>
>
> Am 17.11.25 um 14:17 schrieb Michael Köppl:
>> This avoids sending the parameter with the default value when it's not
>> necessary, since the API already defines the param as optional and
>> assigns a default value of 0.
>
> Why does this work though, 0 and undefined seem close enough for anybody
> without in-depth knowledge about ExtJS and/or our widget-toolkit.
>
> Would be nice to reference upstream docs for chechbox's uncheckedValue
> config parameter, which plays the core part here:
>
>> By default this is undefined, which results in nothing being submitted
>> for the checkbox field when the form is submitted
>
> -- https://docs.sencha.com/extjs/7.0.0/classic/Ext.form.field.Checkbox.html#cfg-uncheckedValue
>
>>
>> Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
>> ---
>> Thanks to @Friedrich for bringing this to my attention. A case where
>> this occurs is in clusters where at least one of the nodes does not yet
>> support the ha-managed param and a VM is created on that node from the
>> web UI of a node that already supports it. Although in general it can
>> probably be expected that all nodes should be upgraded to a new version,
>> this avoids failing the CreateWizard dialog entirely for cases where the
>> "Add to HA" checkbox wasn't even checked at basically no cost because
>> the API already defines a default value of 0 and considers the param
>> optional.
>>
>> www/manager6/lxc/CreateWizard.js | 4 ++--
>> www/manager6/qemu/CreateWizard.js | 4 ++--
>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/www/manager6/lxc/CreateWizard.js b/www/manager6/lxc/CreateWizard.js
>> index f35709f14..024536c58 100644
>> --- a/www/manager6/lxc/CreateWizard.js
>> +++ b/www/manager6/lxc/CreateWizard.js
>> @@ -85,8 +85,8 @@ Ext.define('PVE.lxc.CreateWizard', {
>> {
>> xtype: 'proxmoxcheckbox',
>> name: 'ha-managed',
>> - uncheckedValue: 0,
>> - defaultValue: 0,
>> + uncheckedValue: undefined,
>> + defaultValue: undefined,
>
> Both are the default for (proxmox)checkbox, so could be just omitted? Or if, then
> I'd just set "uncheckedValue" explicitly and add a small comment there so that it's
> a bit more explicit and telling.
>
>> fieldLabel: gettext('Add to HA'),
>> },
>> ],
>> diff --git a/www/manager6/qemu/CreateWizard.js b/www/manager6/qemu/CreateWizard.js
>> index e0c56bc0b..564ca1176 100644
>> --- a/www/manager6/qemu/CreateWizard.js
>> +++ b/www/manager6/qemu/CreateWizard.js
>> @@ -99,8 +99,8 @@ Ext.define('PVE.qemu.CreateWizard', {
>> {
>> xtype: 'proxmoxcheckbox',
>> name: 'ha-managed',
>> - uncheckedValue: 0,
>> - defaultValue: 0,
>> + uncheckedValue: undefined,
>> + defaultValue: undefined,
>
> Same here.
>
>> fieldLabel: gettext('Add to HA'),
>> },
>> ],
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-11-17 17:35 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-17 13:18 [pve-devel] [PATCH manager 1/1] fix: ha: set ha-managed param default and unchecked value to undefined Michael Köppl
2025-11-17 16:47 ` Thomas Lamprecht
2025-11-17 17:35 ` Michael Köppl
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox