[pbs-devel] [PATCH proxmox-backup] ui: prune job edit: fix disabling jobs

[pbs-devel] [PATCH proxmox-backup] ui: prune job edit: fix disabling jobs

[Fiona Ebner]

The backend doesn't have an 'enable' option, but 'disable'. Could be
converted before sending and after loading, but it's cleaner to just
align it with the backend.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 www/window/PruneJobEdit.js | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/www/window/PruneJobEdit.js b/www/window/PruneJobEdit.js
index 87be71ad..1b43b31f 100644
--- a/www/window/PruneJobEdit.js
+++ b/www/window/PruneJobEdit.js
@@ -112,11 +112,11 @@ Ext.define('PBS.window.PruneJobEdit', {
 	    },
 	    {
 		xtype: 'proxmoxcheckbox',
-		fieldLabel: gettext('Enabled'),
-		name: 'enable',
-		uncheckedValue: 0,
-		defaultValue: 1,
-		checked: true,
+		fieldLabel: gettext('Disabled'),
+		name: 'disable',
+		uncheckedValue: false,
+		defaultValue: true,
+		checked: false,
 	    },
 	],
 
-- 
2.30.2

Re: [pbs-devel] [PATCH proxmox-backup] ui: prune job edit: fix disabling jobs

[Dominik Csapak]

IMO good, but do we really drop unrecognized parameters in the api?
is that intentional? (iow. in pve it would have triggered an api exception
since that parameter does not exist)

Re: [pbs-devel] [PATCH proxmox-backup] ui: prune job edit: fix disabling jobs

[Fabian Grünbichler]

On November 14, 2022 11:54 am, Dominik Csapak wrote:
> IMO good, but do we really drop unrecognized parameters in the api?
> is that intentional? (iow. in pve it would have triggered an api exception
> since that parameter does not exist)

yes, if it's an AllOf Schema (which everything containing a flattened type is).
for regular ObjectSchema-s, AdditionalProperties like in PVE is configurable.

Re: [pbs-devel] [PATCH proxmox-backup] ui: prune job edit: fix disabling jobs

[Dominik Csapak]

On 11/14/22 14:26, Fabian Grünbichler wrote:
> On November 14, 2022 11:54 am, Dominik Csapak wrote:
>> IMO good, but do we really drop unrecognized parameters in the api?
>> is that intentional? (iow. in pve it would have triggered an api exception
>> since that parameter does not exist)
> 
> yes, if it's an AllOf Schema (which everything containing a flattened type is).
> for regular ObjectSchema-s, AdditionalProperties like in PVE is configurable.
> 

meh... for configuration APIs i quite like the PVE behavior, since we can catch
such things much earlier.

Re: [pbs-devel] [PATCH proxmox-backup] ui: prune job edit: fix disabling jobs

[Thomas Lamprecht]

Am 14/11/2022 um 10:42 schrieb Fiona Ebner:
> The backend doesn't have an 'enable' option, but 'disable'. Could be
> converted before sending and after loading, but it's cleaner to just
> align it with the backend.

while it is cleaner code-wise, its worse UX-wise; as checking (=enabling) a
"negative" option like disable is always a bit odd and in general a bit less
intuitive.

I replaced this by a patch that converts it on load/before send to keep the
"Enable" display.

Re: [pbs-devel] [PATCH proxmox-backup] ui: prune job edit: fix disabling jobs

[Fiona Ebner]

Am 15.11.22 um 15:29 schrieb Thomas Lamprecht:
> Am 14/11/2022 um 10:42 schrieb Fiona Ebner:
>> The backend doesn't have an 'enable' option, but 'disable'. Could be
>> converted before sending and after loading, but it's cleaner to just
>> align it with the backend.
> 
> while it is cleaner code-wise, its worse UX-wise; as checking (=enabling) a
> "negative" option like disable is always a bit odd and in general a bit less
> intuitive.
> 
> I replaced this by a patch that converts it on load/before send to keep the
> "Enable" display.

Makes sense, thanks! For PBS 3.0, should we align it with sync and
verify jobs where the schedule is optional (so disabled if no schedule
is set)? That would close #4342.