i could not apply 2/3 so i looked only at the gui for now
looks mostly good, some comments inline
On 6/19/19 12:08 PM, Tim Marx wrote:
Signed-off-by: Tim Marx <t.m...@proxmox.com>
---
www/manager6/dc/Backup.js | 77 +++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 71 insertions(+), 6 deletions(-)
diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index c056a647..34052746 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -31,7 +31,8 @@ Ext.define('PVE.dc.BackupEdit', {
comboItems: [
['include', gettext('Include selected VMs')],
['all', gettext('All')],
- ['exclude', gettext('Exclude selected VMs')]
+ ['exclude', gettext('Exclude selected VMs')],
+ ['pool', gettext('Pool')]
],
fieldLabel: gettext('Selection mode'),
name: 'selMode',
@@ -111,6 +112,33 @@ Ext.define('PVE.dc.BackupEdit', {
]
});
+ var selectPoolMembers = function(poolid) {
+ if (!poolid) {
+ return;
+ }
+ sm.deselectAll(true);
+ store.filter([
+ {
+ id: 'poolFilter',
+ property: 'pool',
+ value: poolid
+ }
+ ]);
+ sm.selectAll(true);
+ };
+
+ var selPool = Ext.create('PVE.form.PoolSelector', {
+ fieldLabel: gettext('Pool'),
+ hidden: true,
+ allowBlank: true,
+ name: 'pool',
+ listeners: {
+ change: function( selpool, newValue, oldValue) {
+ selectPoolMembers(newValue);
+ }
+ }
+ });
+
var nodesel = Ext.create('PVE.form.NodeSelector', {
name: 'node',
fieldLabel: gettext('Node'),
@@ -129,6 +157,10 @@ Ext.define('PVE.dc.BackupEdit', {
if (mode === 'all') {
sm.selectAll(true);
}
+
+ if (mode === 'pool') {
+ selectPoolMembers(selPool.value);
+ }
}
}
});
@@ -153,7 +185,8 @@ Ext.define('PVE.dc.BackupEdit', {
value: '00:00',
allowBlank: false
},
- selModeField
+ selModeField,
+ selPool
];
var column2 = [
@@ -217,13 +250,19 @@ Ext.define('PVE.dc.BackupEdit', {
values.all = 1;
values.exclude = values.vmid;
delete values.vmid;
+ } else if (selMode === 'pool') {
+ delete values.vmid;
+ }
+
+ if (selMode !== 'pool') {
+ delete values.pool;
}
return values;
}
});
var update_vmid_selection = function(list, mode) {
- if (mode !== 'all') {
+ if (mode !== 'all' && mode !== 'pool') {
sm.deselectAll(true);
if (list) {
Ext.Array.each(list.split(','), function(vmid) {
@@ -242,15 +281,32 @@ Ext.define('PVE.dc.BackupEdit', {
});
selModeField.on('change', function(f, value, oldValue) {
+ if (oldValue === 'pool') {
+ store.removeFilter('poolFilter');
+ }
+
+ if (oldValue === 'all') {
+ sm.deselectAll(true);
+ vmidField.setValue('');
+ }
+
if (value === 'all') {
sm.selectAll(true);
vmgrid.setDisabled(true);
} else {
vmgrid.setDisabled(false);
}
- if (oldValue === 'all') {
- sm.deselectAll(true);
+
+ if (value === 'pool') {
+ vmgrid.setDisabled(true);
vmidField.setValue('');
+ selPool.setVisible(true);
+ selPool.allowBlank = false;
+ selectPoolMembers(selPool.value);
+
+ } else {
+ selPool.setVisible(false);
+ selPool.allowBlank = true;
}
var list = vmidField.getValue();
update_vmid_selection(list, value);
while this hunk is not wrong, it looks a little complicated
because you set the same things multiple times (e.g. vmgrid)
since we have here a limited set of modes
(afaics 'all', 'pool', 'include', 'exclude')
why not do a big
switch(oldValue):
case 'foo'
case 'bar'
block and one for the new value?
i think this would greatly improve the readability,
since one can see instantly what happens in each case
@@ -269,6 +325,8 @@ Ext.define('PVE.dc.BackupEdit', {
var mode = selModeField.getValue();
if (mode === 'all') {
sm.selectAll(true);
+ } else if (mode === 'pool'){
+ selectPoolMembers(selPool.value);
} else {
update_vmid_selection(list, mode);
}
@@ -302,6 +360,9 @@ Ext.define('PVE.dc.BackupEdit', {
data.vmid = '';
data.selMode = 'all';
}
+ } else if (data.pool) {
+ data.selMode = 'pool';
+ data.selPool = data.pool;
} else {
data.selMode = 'include';
}
two things on this hunk:
first i do not think you need to set 'data.selPool' since
the name of the field is 'pool' and that is set already
second, i know it already was this way, but would it not make more
sense to split the first 'if' to a 'if/else if' ?
e.g.
if (data.all)
// do something
else if (data.exclude)
// do something else
else if (data.pool)
...
etc.
this can of course be done as a cleanup commit sometime after
and not really related to your patch (i only say it since you touch the
lines ;) )
@@ -490,6 +551,10 @@ Ext.define('PVE.dc.BackupView', {
return record.data.vmid;
}
+ if (record.data.pool) {
+ return record.data.pool;
+ }
+
return "-";
}
}
@@ -509,7 +574,7 @@ Ext.define('PVE.dc.BackupView', {
fields: [
'id', 'starttime', 'dow',
'storage', 'node', 'vmid', 'exclude',
- 'mailto',
+ 'mailto', 'pool',
{ name: 'enabled', type: 'boolean' },
{ name: 'all', type: 'boolean' },
{ name: 'snapshot', type: 'boolean' },
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
