Re: [pve-devel] [PATCH manager 1/1] fix: ha: set ha-managed param default and unchecked value to undefined

["Michael Köppl" <m.koeppl@proxmox.com>]

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