[pve-devel] [PATCH manager 0/5 v2] BackupView as TreePanel

[pve-devel] [PATCH manager 0/5 v2] BackupView as TreePanel

[Matthias Heiserer]

This replaces the backupview for storages and the BackupView for
qemu and lxc with a single TreeView.
Less code, no need to maintain two different BackupView files,
and reduces complexity of ContentView.

Changes from v1:
- replace grid/BackupView with storage/BackupView
- some bug fixes and code cleanup
- group unknown/renamed backups in the folder "Other"
- remember expanded nodes and selection

Matthias Heiserer (5):
  Storage GUI: Rewrite backup content view as TreePanel.
  GUI: Utils: Helpers for backup type and icon
  Backup GUI: Use the new storage/BackupView instead of grid/BackupView.
  Remove grid/backupview as it got replaced by storage/backupview
  Storage ContentView: remove code that was required only for backupview

 www/manager6/Utils.js               |  20 +
 www/manager6/grid/BackupView.js     | 388 ---------------
 www/manager6/lxc/Config.js          |   2 +-
 www/manager6/qemu/Config.js         |   2 +-
 www/manager6/storage/BackupView.js  | 702 ++++++++++++++++++++--------
 www/manager6/storage/ContentView.js |  22 +-
 6 files changed, 535 insertions(+), 601 deletions(-)
 delete mode 100644 www/manager6/grid/BackupView.js

-- 
2.30.2

[pve-devel] [PATCH manager 1/5 v2] Storage GUI: Rewrite backup content view as TreePanel.

[Matthias Heiserer]

Should be easier to read/use than the current flat list.
Backups are grouped by ID and type, so in case there are backups
with ID 100 for both CT and VM, this would create two separate
groups in the UI.
Date and size of group are taken from the latest backup.
Notes, Protection, Encrypted, and Verify State stay as default
value empty, empty, No, and None, respectively.

Code adapted from the existing backup view and the pbs
datastore content, where appropriate.

Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
---
Changes from v1: 

-add BackupNow button to create backups when not used for storage

-Display combined size of backups for groups, except for PBS where
deduplication happens

-remove timeout for API call

-inform window/restore if storage is PBS

-use get_backup_type helper

-some bug fixes, e.g. text was used instead of volid

-remove groupField, doesn't do anything for TreeStore

-only sort by text, ie. `(qemu|lxc)/<VMID>`

-use render_storage_content helper for displaying backup group

-remove content from store as it's never used. everything is
 content-type backup because we get the data from API
 
-group by backuptype rather than format, so that e.g tar and tar.zst
 are in the same group.
 
-code cleanup: reorder statements, remove one-character variable,
 remove debug log, rename this to me, change filtering, rename
 notPBS to isPBS
 
-add text field to model. fix date field in model

-remember expanded nodes and selection

-rename backup model

-use filterer only with correct store

 
 www/manager6/storage/BackupView.js | 702 +++++++++++++++++++++--------
 1 file changed, 511 insertions(+), 191 deletions(-)

diff --git a/www/manager6/storage/BackupView.js b/www/manager6/storage/BackupView.js
index 2328c0fc..cbb1624b 100644
--- a/www/manager6/storage/BackupView.js
+++ b/www/manager6/storage/BackupView.js
@@ -1,223 +1,543 @@
-Ext.define('PVE.storage.BackupView', {
-    extend: 'PVE.storage.ContentView',
+Ext.define('PVE.storage.BackupModel', {
+    extend: 'Ext.data.Model',
+    fields: [
+	{
+	    name: 'ctime',
+	    type: 'date',
+	    dateFormat: 'timestamp',
+	},
+	'format',
+	'volid',
+	'vmid',
+	'size',
+	'protected',
+	'notes',
+	'text',
+    ],
+});
 
+
+Ext.define('PVE.storage.BackupView', {
+    extend: 'Ext.tree.Panel',
     alias: 'widget.pveStorageBackupView',
+    mixins: ['Proxmox.Mixin.CBind'],
+    rootVisible: false,
 
-    showColumns: ['name', 'notes', 'protected', 'date', 'format', 'size'],
+    title: gettext('Content'),
 
-    initComponent: function() {
-	var me = this;
+    cbindData: function(initialCfg) {
+	this.isPBS = initialCfg.pluginType === 'pbs';
+	return {};
+    },
+    isStorage: false,
 
-	var nodename = me.nodename = me.pveSelNode.data.node;
-	if (!nodename) {
-	    throw "no node name specified";
-	}
+    controller: {
+	xclass: 'Ext.app.ViewController',
 
-	var storage = me.storage = me.pveSelNode.data.storage;
-	if (!storage) {
-	    throw "no storage ID specified";
-	}
+	groupnameHelper: function(item) {
+	    if (item.vmid) {
+		return PVE.Utils.get_backup_type(item.volid, item.format) + `/${item.vmid}`;
+	    } else {
+		return 'Other';
+	    }
+	},
 
-	me.content = 'backup';
+	init: function(view) {
+	    let me = this;
+	    me.storage = view?.pveSelNode?.data?.storage;
+	    me.nodename = view.nodename || view.pveSelNode.data.node;
+	    me.vmid = view.pveSelNode.data.vmid;
+	    me.vmtype = view.pveSelNode.data.type;
 
-	var sm = me.sm = Ext.create('Ext.selection.RowModel', {});
+	    me.store = Ext.create('Ext.data.Store', {
+		model: 'PVE.storage.BackupModel',
+	    });
+	    me.store.on('load', me.onLoad, me);
+	    view.getStore().setConfig('filterer', 'bottomup');
+	    view.getStore().setSorters(['text']);
 
-	var reload = function() {
-	    me.store.load();
-	};
+	    if (me.vmid) {
+		me.getView().getStore().filter({
+		    property: 'vmid',
+		    value: me.vmid,
+		    exactMatch: true,
+		});
+	    } else {
+		me.lookup('storagesel').setVisible(false);
+		me.lookup('backupNowButton').setVisible(false);
+	    }
+	    Proxmox.Utils.monStoreErrors(view, me.store);
+	},
 
-	let pruneButton = Ext.create('Proxmox.button.Button', {
-	    text: gettext('Prune group'),
-	    disabled: true,
-	    selModel: sm,
-	    setBackupGroup: function(backup) {
-		if (backup) {
-		    let name = backup.text;
-		    let vmid = backup.vmid;
-		    let format = backup.format;
+	onLoad: function(store, records, success, operation) {
+	    let me = this;
+	    let view = me.getView();
+	    let selection = view.getSelection()?.[0];
+	    selection = selection?.parentNode?.data?.text +selection?.data?.volid;
 
-		    let vmtype;
-		    if (name.startsWith('vzdump-lxc-') || format === "pbs-ct") {
-			vmtype = 'lxc';
-		    } else if (name.startsWith('vzdump-qemu-') || format === "pbs-vm") {
-			vmtype = 'qemu';
+	    let expanded = {};
+	    view.getRootNode().cascadeBy({
+		before: item => {
+		    if (item.isExpanded() && !item.data.leaf) {
+			let id = item.data.text;
+			expanded[id] = true;
+			return true;
 		    }
+		    return false;
+		},
+		after: Ext.emptyFn,
+	    });
+	    let groups = me.getRecordGroups(records, expanded);
 
-		    if (vmid && vmtype) {
-			this.setText(gettext('Prune group') + ` ${vmtype}/${vmid}`);
-			this.vmid = vmid;
-			this.vmtype = vmtype;
-			this.setDisabled(false);
-			return;
-		    }
+	    for (const item of records.map(i => i.data)) {
+		item.text = item.volid;
+		item.leaf = true;
+		item.iconCls = 'fa-file-o';
+		groups[me.groupnameHelper(item)].children.push(item);
+		groups[me.groupnameHelper(item)].size += item.size;
+	    }
+
+	    for (let [_name, group] of Object.entries(groups)) {
+		let children = group.children;
+		let latest = children.reduce((l, r) => l.ctime > r.ctime ? l : r);
+		group.ctime = latest.ctime;
+		if (view.isPBS) {
+		    group.size = latest.size;
 		}
-		this.setText(gettext('Prune group'));
-		this.vmid = null;
-		this.vmtype = null;
-		this.setDisabled(true);
-	    },
-	    handler: function(b, e, rec) {
-		let win = Ext.create('PVE.window.Prune', {
-		    nodename: nodename,
-		    storage: storage,
-		    backup_id: this.vmid,
-		    backup_type: this.vmtype,
+		let num_verified = children.reduce((l, r) => l + r.verification === 'ok', 0);
+		group.verified = num_verified / children.length;
+	    }
+
+	    let children = [];
+	    Object.entries(groups).forEach(e => children.push(e[1]));
+	    view.setRootNode({
+		expanded: true,
+		children: children,
+	    });
+
+	    if (selection) {
+		let rootnode = view.getRootNode();
+		let selected;
+		rootnode.cascade(node => {
+		    if (selected) {return false;} // skip if already found
+		    let id = node.parentNode?.data?.text + node.data?.volid;
+		    if (id === selection) {
+			selected = node;
+			return false;
+		    }
+		    return true;
 		});
-		win.show();
-		win.on('destroy', reload);
-	    },
-	});
+		view.setSelection(selected);
+		view.getView().focusRow(selected);
+	    }
+	    Proxmox.Utils.setErrorMask(view, false);
+	},
 
-	me.on('selectionchange', function(model, srecords, eOpts) {
-	    if (srecords.length === 1) {
-		pruneButton.setBackupGroup(srecords[0].data);
-	    } else {
-		pruneButton.setBackupGroup(null);
+	reload: function() {
+	    let me = this;
+	    let view = me.getView();
+	    if (!view.store || !me.store) {
+		console.warn('cannot reload, no store(s)');
+		return;
 	    }
-	});
-
-	let isPBS = me.pluginType === 'pbs';
-
-	me.tbar = [
-	    {
-		xtype: 'proxmoxButton',
-		text: gettext('Restore'),
-		selModel: sm,
-		disabled: true,
-		handler: function(b, e, rec) {
-		    var vmtype;
-		    if (PVE.Utils.volume_is_qemu_backup(rec.data.volid, rec.data.format)) {
-			vmtype = 'qemu';
-		    } else if (PVE.Utils.volume_is_lxc_backup(rec.data.volid, rec.data.format)) {
-			vmtype = 'lxc';
-		    } else {
-			return;
-		    }
 
-		    var win = Ext.create('PVE.window.Restore', {
-			nodename: nodename,
-			volid: rec.data.volid,
-			volidText: PVE.Utils.render_storage_content(rec.data.volid, {}, rec),
-			vmtype: vmtype,
-			isPBS: isPBS,
-		    });
-		    win.show();
-		    win.on('destroy', reload);
-		},
-	    },
-	];
-	if (isPBS) {
-	    me.tbar.push({
-		xtype: 'proxmoxButton',
-		text: gettext('File Restore'),
-		disabled: true,
-		selModel: sm,
-		handler: function(b, e, rec) {
-		    let isVMArchive = PVE.Utils.volume_is_qemu_backup(rec.data.volid, rec.data.format);
-		    Ext.create('Proxmox.window.FileBrowser', {
-			title: gettext('File Restore') + " - " + rec.data.text,
-			listURL: `/api2/json/nodes/localhost/storage/${me.storage}/file-restore/list`,
-			downloadURL: `/api2/json/nodes/localhost/storage/${me.storage}/file-restore/download`,
-			extraParams: {
-			    volume: rec.data.volid,
-			},
-			archive: isVMArchive ? 'all' : undefined,
-			autoShow: true,
-		    });
+	    if (!me.storage) {
+		Proxmox.Utils.setErrorMask(view, true);
+		return;
+	    }
+
+	    let url = `/api2/json/nodes/${me.nodename}/storage/${me.storage}/content`;
+	    me.store.setProxy({
+		type: 'proxmox',
+		url: url,
+		extraParams: {
+		    content: 'backup',
 		},
 	    });
-	}
-	me.tbar.push(
-	    {
-		xtype: 'proxmoxButton',
-		text: gettext('Show Configuration'),
-		disabled: true,
-		selModel: sm,
-		handler: function(b, e, rec) {
-		    var win = Ext.create('PVE.window.BackupConfig', {
-			volume: rec.data.volid,
-			pveSelNode: me.pveSelNode,
-		    });
-
-		    win.show();
+
+	    me.store.load();
+	    Proxmox.Utils.monStoreErrors(view, me.store);
+	},
+
+	getRecordGroups: function(records, expanded) {
+	    let groups = {};
+	    for (const item of records) {
+		const groupName = this.groupnameHelper(item.data);
+		groups[groupName] = {
+		    vmid: item.data.vmid,
+		    leaf: false,
+		    children: [],
+		    expanded: !!expanded[groupName],
+		    text: groupName,
+		    ctime: 0,
+		    format: item.data.format,
+		    volid: item.data.volid, // to preserve backup type information
+		    size: 0,
+		    iconCls: PVE.Utils.get_backup_type_icon_cls(item.data.volid, item.data.format),
+		};
+	    }
+	    return groups;
+	},
+
+	restoreHandler: function(button, event, rec) {
+	    let me = this;
+	    let vmtype = PVE.Utils.get_backup_type(rec.data.volid, rec.data.format);
+	    let win = Ext.create('PVE.window.Restore', {
+		nodename: me.nodename,
+		volid: rec.data.volid,
+		volidText: PVE.Utils.render_storage_content(rec.data.volid, {}, rec),
+		vmtype: vmtype,
+		isPBS: me.view.isPBS,
+	    });
+	    win.on('destroy', () => me.reload());
+	    win.show();
+	},
+
+	restoreFilesHandler: function(button, event, rec) {
+	    let me = this;
+	    let isVMArchive = PVE.Utils.volume_is_qemu_backup(rec.data.volid, rec.data.format);
+	    Ext.create('Proxmox.window.FileBrowser', {
+		title: gettext('File Restore') + " - " + rec.data.text,
+		listURL: `/api2/json/nodes/localhost/storage/${me.storage}/file-restore/list`,
+		downloadURL: `/api2/json/nodes/localhost/storage/${me.storage}/file-restore/download`,
+		extraParams: {
+		    volume: rec.data.volid,
 		},
-	    },
-	    {
-		xtype: 'proxmoxButton',
-		text: gettext('Edit Notes'),
-		disabled: true,
-		selModel: sm,
-		handler: function(b, e, rec) {
-		    let volid = rec.data.volid;
-		    Ext.create('Proxmox.window.Edit', {
-			autoLoad: true,
-			width: 600,
-			height: 400,
-			resizable: true,
-			title: gettext('Notes'),
-			url: `/api2/extjs/nodes/${nodename}/storage/${me.storage}/content/${volid}`,
+		archive: isVMArchive ? 'all' : undefined,
+		autoShow: true,
+	    });
+	},
+
+	showConfigurationHandler: function(button, event, rec) {
+	    let win = Ext.create('PVE.window.BackupConfig', {
+		volume: rec.data.volid,
+		pveSelNode: this.view.pveSelNode,
+	    });
+	    win.show();
+	},
+
+	editNotesHandler: function(button, event, rec) {
+	    let me = this;
+	    let volid = rec.data.volid;
+	    Ext.create('Proxmox.window.Edit', {
+		autoLoad: true,
+		width: 600,
+		height: 400,
+		resizable: true,
+		title: gettext('Notes'),
+		url: `/api2/extjs/nodes/${me.nodename}/storage/${me.storage}/content/${volid}`,
+		layout: 'fit',
+		items: [
+		    {
+			xtype: 'textarea',
 			layout: 'fit',
-			items: [
-			    {
-				xtype: 'textarea',
-				layout: 'fit',
-				name: 'notes',
-				height: '100%',
-			    },
-			],
-			listeners: {
-			    destroy: () => reload(),
-			},
-		    }).show();
+			name: 'notes',
+			height: '100%',
+		    },
+		],
+		listeners: {
+		    destroy: () => me.reload(),
 		},
-	    },
-	    {
-		xtype: 'proxmoxButton',
-		text: gettext('Change Protection'),
-		disabled: true,
-		handler: function(button, event, record) {
-		    const volid = record.data.volid;
-		    Proxmox.Utils.API2Request({
-			url: `/api2/extjs/nodes/${nodename}/storage/${me.storage}/content/${volid}`,
-			method: 'PUT',
-			waitMsgTarget: me,
-			params: { 'protected': record.data.protected ? 0 : 1 },
-			failure: (response) => Ext.Msg.alert('Error', response.htmlStatus),
-			success: (response) => reload(),
-		    });
+	    }).show();
+	},
+
+	changeProtectionHandler: function(button, event, rec) {
+	    let me = this;
+	    const volid = rec.data.volid;
+	    Proxmox.Utils.API2Request({
+		url: `/api2/extjs/nodes/${me.nodename}/storage/${me.storage}/content/${volid}`,
+		method: 'PUT',
+		waitMsgTarget: button,
+		params: { 'protected': rec.data.protected ? 0 : 1 },
+		failure: (response) => Ext.Msg.alert('Error', response.htmlStatus),
+		success: (_) => me.reload(),
+	    });
+	},
+
+	pruneGroupHandler: function(button, event, rec) {
+	    let me = this;
+	    let vmtype = PVE.Utils.get_backup_type(rec.data.volid, rec.data.format);
+	    Ext.create('PVE.window.Prune', {
+		nodename: me.nodename,
+		storage: me.storage,
+		backup_id: rec.data.vmid,
+		backup_type: vmtype,
+		rec: rec,
+		listeners: {
+		    destroy: () => me.reload(),
 		},
-	    },
-	    '-',
-	    pruneButton,
-	);
-
-	if (isPBS) {
-	    me.extraColumns = {
-		encrypted: {
-		    header: gettext('Encrypted'),
-		    dataIndex: 'encrypted',
-		    renderer: PVE.Utils.render_backup_encryption,
+	    }).show();
+	},
+
+	removeHandler: function(button, event, rec) {
+	    let me = this;
+	    const volid = rec.data.volid;
+	    Proxmox.Utils.API2Request({
+		url: `/nodes/${me.nodename}/storage/${me.storage}/content//${volid}`,
+		method: 'DELETE',
+		callback: () => me.reload(),
+		failure: response => Ext.Msg.alert(gettext('Error'), response.htmlStatus),
+	    });
+	},
+
+	searchKeyupFn: function(field) {
+	    let me = this;
+	    me.getView().getStore().getFilters().removeByKey('volid');
+	    me.getView().getStore().filter([
+		{
+		    property: 'volid',
+		    value: field.getValue(),
+		    anyMatch: true,
+		    caseSensitive: false,
+		    id: 'volid',
 		},
-		verification: {
-		    header: gettext('Verify State'),
-		    dataIndex: 'verification',
-		    renderer: PVE.Utils.render_backup_verification,
+	    ]);
+	},
+
+	searchClearHandler: function(field) {
+	    field.triggers.clear.setVisible(false);
+	    field.setValue(this.originalValue);
+	    this.getView().getStore().clearFilter();
+	},
+
+	searchChangeFn: function(field, newValue, oldValue) {
+	    if (newValue !== field.originalValue) {
+		field.triggers.clear.setVisible(true);
+	    }
+	},
+
+	storageSelectorBoxReady: function(selector, width, height, eOpts) {
+	    selector.setNodename(this.nodename);
+	},
+
+	storageSelectorChange: function(self, newValue, oldValue, eOpts) {
+	    let me = this;
+	    me.storage = newValue;
+	    me.getView().getSelectionModel().deselectAll();
+	    me.reload();
+	},
+
+	backupNowHandler: function(button, event) {
+	    let me = this;
+	    Ext.create('PVE.window.Backup', {
+		nodename: me.nodename,
+		vmid: me.vmid,
+		vmtype: me.vmtype,
+		storage: me.storage,
+		listeners: {
+		    close: () => me.reload(),
 		},
-	    };
-	}
+	    }).show();
+	},
+    },
+
+    columns: [
+	{
+	    xtype: 'treecolumn',
+	    header: gettext("Backup Group"),
+	    dataIndex: 'text',
+	    renderer: function(value, _metadata, record) {
+		if (record.phantom) { return value; }
+		return PVE.Utils.render_storage_content(...arguments);
+	    },
+	    flex: 2,
+	},
+	{
+	    header: gettext('Notes'),
+	    flex: 1,
+	    renderer: Ext.htmlEncode,
+	    dataIndex: 'notes',
+	},
+	{
+	    header: `<i class="fa fa-shield"></i>`,
+	    tooltip: gettext('Protected'),
+	    width: 30,
+	    renderer: v => v ? `<i data-qtip="${gettext('Protected')}" class="fa fa-shield"></i>` : '',
+	    sorter: (a, b) => (b.data.protected || 0) - (a.data.protected || 0),
+	    dataIndex: 'protected',
+	},
+	{
+	    header: gettext('Date'),
+	    width: 150,
+	    dataIndex: 'ctime',
+	    xtype: 'datecolumn',
+	    format: 'Y-m-d H:i:s',
+	},
+	{
+	    header: gettext('Format'),
+	    width: 100,
+	    dataIndex: 'format',
+	},
+	{
+	    header: gettext('Size'),
+	    width: 100,
+	    renderer: Proxmox.Utils.format_size,
+	    dataIndex: 'size',
+	},
+	{
+	    header: gettext('Encrypted'),
+	    dataIndex: 'encrypted',
+	    renderer: PVE.Utils.render_backup_encryption,
+	    cbind: {
+		hidden: '{!isPBS}',
+	    },
+	},
+	{
+	    header: gettext('Verify State'),
+	    dataIndex: 'verification',
+	    renderer: PVE.Utils.render_backup_verification,
+	    cbind: {
+		hidden: '{!isPBS}',
+	    },
+	},
+    ],
+
+    tbar: [
+	{
+	    xtype: 'button',
+	    text: gettext('Backup now'),
+	    handler: 'backupNowHandler',
+	    reference: 'backupNowButton',
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Restore'),
+	    handler: 'restoreHandler',
+	    parentXType: "treepanel",
+	    disabled: true,
+	    enableFn: record => record.phantom === false,
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('File Restore'),
+	    handler: 'restoreFilesHandler',
+	    cbind: {
+		hidden: '{!isPBS}',
+	    },
+	    parentXType: "treepanel",
+	    disabled: true,
+	    enableFn: record => record.phantom === false,
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Show Configuration'),
+	    handler: 'showConfigurationHandler',
+	    parentXType: "treepanel",
+	    disabled: true,
+	    enableFn: record => record.phantom === false,
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Edit Notes'),
+	    handler: 'editNotesHandler',
+	    parentXType: "treepanel",
+	    disabled: true,
+	    enableFn: record => record.phantom === false,
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Change Protection'),
+	    handler: 'changeProtectionHandler',
+	    parentXType: "treepanel",
+	    disabled: true,
+	    enableFn: record => record.phantom === false,
+	},
+	'-',
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Prune group'),
+	    setBackupGroup: function(backup) {
+		let me = this;
+		if (backup) {
+		    let volid = backup.volid;
+		    let vmid = backup.vmid;
+		    let format = backup.format;
 
-	me.callParent();
+		    let vmtype = PVE.Utils.get_backup_type(volid, format);
+		    if (vmid && vmtype) {
+			me.setText(gettext('Prune group') + ` ${vmtype}/${vmid}`);
+			me.vmid = vmid;
+			me.vmtype = vmtype;
+			me.setDisabled(false);
+			return;
+		    }
+		}
+		me.setText(gettext('Prune group'));
+		me.vmid = null;
+		me.vmtype = null;
+		me.setDisabled(true);
+	    },
+	    handler: 'pruneGroupHandler',
+	    parentXType: "treepanel",
+	    disabled: true,
+	    reference: 'pruneButton',
+	    enableFn: () => true,
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Remove'),
+	    handler: 'removeHandler',
+	    parentXType: 'treepanel',
+	    disabled: true,
+	    enableFn: record => record.phantom === false && !record?.data?.protected,
+	    confirmMsg: function(rec) {
+		let name = rec.data.text;
+		return Ext.String.format(gettext('Are you sure you want to remove entry {0}'), `'${name}'`);
+	    },
+	},
+	'->',
+	{
+	    xtype: 'pveStorageSelector',
+	    fieldLabel: gettext('Storage'),
+	    storageContent: 'backup',
+	    reference: 'storagesel',
+	    listeners: {
+		change: 'storageSelectorChange',
+		boxready: 'storageSelectorBoxReady',
+	    },
 
-	me.store.getSorters().clear();
-	me.store.setSorters([
-	    {
-		property: 'vmid',
-		direction: 'ASC',
+	},
+	gettext('Search') + ':',
+	' ',
+	{
+	    xtype: 'textfield',
+	    width: 200,
+	    enableKeyEvents: true,
+	    emptyText: gettext('Name, Format'),
+	    listeners: {
+		keyup: {
+		    buffer: 500,
+		    fn: 'searchKeyupFn',
+		},
+		change: 'searchChangeFn',
 	    },
-	    {
-		property: 'vdate',
-		direction: 'DESC',
+	    triggers: {
+		clear: {
+		    cls: 'pmx-clear-trigger',
+		    weight: -1,
+		    hidden: true,
+		    handler: 'searchClearHandler',
+		},
 	    },
-	]);
+	},
+	],
+
+    listeners: {
+	activate: function() {
+	    let me = this;
+	    // only load on first activate to not load every tab switch
+	    if (!me.firstLoad) {
+		me.getController().reload();
+		me.firstLoad = true;
+	    }
+	},
+	selectionchange: function(model, srecords, eOpts) {
+	    let pruneButton = this.getController().lookup('pruneButton');
+	    if (srecords.length === 1) {
+		pruneButton.setBackupGroup(srecords[0].data);
+	    } else {
+		pruneButton.setBackupGroup(null);
+	    }
+	},
     },
 });
-- 
2.30.2

[pve-devel] [PATCH manager 2/5 v2] GUI: Utils: Helpers for backup type and icon

[Matthias Heiserer]

Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
---
changes from v1:
add "backup" to name
return empty string instead of throwing
 www/manager6/Utils.js | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index aafe359a..337ccfae 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1803,6 +1803,26 @@ Ext.define('PVE.Utils', {
 
 	return undefined;
     },
+
+    get_backup_type_icon_cls: function(volid, format) {
+	var cls = '';
+	if (PVE.Utils.volume_is_qemu_backup(volid, format)) {
+	    cls = 'fa-desktop';
+	} else if (PVE.Utils.volume_is_lxc_backup(volid, format)) {
+	    cls = 'fa-cube';
+	}
+	return cls;
+    },
+
+    get_backup_type: function(volid, format) {
+	if (PVE.Utils.volume_is_qemu_backup(volid, format)) {
+	    return 'qemu';
+	} else if (PVE.Utils.volume_is_lxc_backup(volid, format)) {
+	    return 'lxc';
+	} else {
+	    return '';
+	}
+    },
 },
 
     singleton: true,
-- 
2.30.2

[pve-devel] [PATCH manager 3/5 v2] Backup GUI: Use the new storage/BackupView instead of grid/BackupView.

[Matthias Heiserer]

Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
---
 www/manager6/lxc/Config.js  | 2 +-
 www/manager6/qemu/Config.js | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/www/manager6/lxc/Config.js b/www/manager6/lxc/Config.js
index 89b59c9b..242780c8 100644
--- a/www/manager6/lxc/Config.js
+++ b/www/manager6/lxc/Config.js
@@ -256,7 +256,7 @@ Ext.define('PVE.lxc.Config', {
 	    me.items.push({
 		title: gettext('Backup'),
 		iconCls: 'fa fa-floppy-o',
-		xtype: 'pveBackupView',
+		xtype: 'pveStorageBackupView',
 		itemId: 'backup',
 	    },
 	    {
diff --git a/www/manager6/qemu/Config.js b/www/manager6/qemu/Config.js
index 9fe933df..3ed2427a 100644
--- a/www/manager6/qemu/Config.js
+++ b/www/manager6/qemu/Config.js
@@ -291,7 +291,7 @@ Ext.define('PVE.qemu.Config', {
 	    me.items.push({
 		title: gettext('Backup'),
 		iconCls: 'fa fa-floppy-o',
-		xtype: 'pveBackupView',
+		xtype: 'pveStorageBackupView',
 		itemId: 'backup',
 	    },
 	    {
-- 
2.30.2

[pve-devel] [PATCH manager 4/5 v2] Remove grid/backupview as it got replaced by storage/backupview

[Matthias Heiserer]

Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
---
 www/manager6/grid/BackupView.js | 388 --------------------------------
 1 file changed, 388 deletions(-)
 delete mode 100644 www/manager6/grid/BackupView.js

diff --git a/www/manager6/grid/BackupView.js b/www/manager6/grid/BackupView.js
deleted file mode 100644
index 7f7e1b62..00000000
--- a/www/manager6/grid/BackupView.js
+++ /dev/null
@@ -1,388 +0,0 @@
-Ext.define('PVE.grid.BackupView', {
-    extend: 'Ext.grid.GridPanel',
-
-    alias: ['widget.pveBackupView'],
-
-    onlineHelp: 'chapter_vzdump',
-
-    stateful: true,
-    stateId: 'grid-guest-backup',
-
-    initComponent: function() {
-	var me = this;
-
-	var nodename = me.pveSelNode.data.node;
-	if (!nodename) {
-	    throw "no node name specified";
-	}
-
-	var vmid = me.pveSelNode.data.vmid;
-	if (!vmid) {
-	    throw "no VM ID specified";
-	}
-
-	var vmtype = me.pveSelNode.data.type;
-	if (!vmtype) {
-	    throw "no VM type specified";
-	}
-
-	var vmtypeFilter;
-	if (vmtype === 'lxc' || vmtype === 'openvz') {
-	    vmtypeFilter = function(item) {
-		return PVE.Utils.volume_is_lxc_backup(item.data.volid, item.data.format);
-	    };
-	} else if (vmtype === 'qemu') {
-	    vmtypeFilter = function(item) {
-		return PVE.Utils.volume_is_qemu_backup(item.data.volid, item.data.format);
-	    };
-	} else {
-	    throw "unsupported VM type '" + vmtype + "'";
-	}
-
-	var searchFilter = {
-	    property: 'volid',
-	    value: '',
-	    anyMatch: true,
-	    caseSensitive: false,
-	};
-
-	var vmidFilter = {
-	    property: 'vmid',
-	    value: vmid,
-	    exactMatch: true,
-	};
-
-	me.store = Ext.create('Ext.data.Store', {
-	    model: 'pve-storage-content',
-	    sorters: [
-		{
-		    property: 'vmid',
-		    direction: 'ASC',
-		},
-		{
-		    property: 'vdate',
-		    direction: 'DESC',
-		},
-	    ],
-	    filters: [
-	        vmtypeFilter,
-		searchFilter,
-		vmidFilter,
-		],
-	});
-
-	let updateFilter = function() {
-	    me.store.filter([
-		vmtypeFilter,
-		searchFilter,
-		vmidFilter,
-	    ]);
-	};
-
-	var reload = Ext.Function.createBuffered(function() {
-	    if (me.store) {
-		me.store.load();
-	    }
-	}, 100);
-
-	let isPBS = false;
-	var setStorage = function(storage) {
-	    var url = '/api2/json/nodes/' + nodename + '/storage/' + storage + '/content';
-	    url += '?content=backup';
-
-	    me.store.setProxy({
-		type: 'proxmox',
-		url: url,
-	    });
-
-	    Proxmox.Utils.monStoreErrors(me.view, me.store, true);
-
-	    reload();
-	};
-
-	let file_restore_btn;
-
-	var storagesel = Ext.create('PVE.form.StorageSelector', {
-	    nodename: nodename,
-	    fieldLabel: gettext('Storage'),
-	    labelAlign: 'right',
-	    storageContent: 'backup',
-	    allowBlank: false,
-	    listeners: {
-		change: function(f, value) {
-		    let storage = f.getStore().findRecord('storage', value, 0, false, true, true);
-		    if (storage) {
-			isPBS = storage.data.type === 'pbs';
-			me.getColumns().forEach((column) => {
-			    let id = column.dataIndex;
-			    if (id === 'verification' || id === 'encrypted') {
-				column.setHidden(!isPBS);
-			    }
-			});
-		    } else {
-			isPBS = false;
-		    }
-		    setStorage(value);
-		    if (file_restore_btn) {
-			file_restore_btn.setHidden(!isPBS);
-		    }
-		},
-	    },
-	});
-
-	var storagefilter = Ext.create('Ext.form.field.Text', {
-	    fieldLabel: gettext('Search'),
-	    labelWidth: 50,
-	    labelAlign: 'right',
-	    enableKeyEvents: true,
-	    value: searchFilter.value,
-	    listeners: {
-		buffer: 500,
-		keyup: function(field) {
-		    me.store.clearFilter(true);
-		    searchFilter.value = field.getValue();
-		    updateFilter();
-		},
-	    },
-	});
-
-	var vmidfilterCB = Ext.create('Ext.form.field.Checkbox', {
-	    boxLabel: gettext('Filter VMID'),
-	    value: '1',
-	    listeners: {
-		change: function(cb, value) {
-		    vmidFilter.value = value ? vmid : '';
-		    vmidFilter.exactMatch = !!value;
-		    updateFilter();
-		},
-	    },
-	});
-
-	var sm = Ext.create('Ext.selection.RowModel', {});
-
-	var backup_btn = Ext.create('Ext.button.Button', {
-	    text: gettext('Backup now'),
-	    handler: function() {
-		var win = Ext.create('PVE.window.Backup', {
-		    nodename: nodename,
-		    vmid: vmid,
-		    vmtype: vmtype,
-		    storage: storagesel.getValue(),
-		    listeners: {
-			close: function() {
-			    reload();
-			},
-		    },
-		});
-		win.show();
-	    },
-	});
-
-	var restore_btn = Ext.create('Proxmox.button.Button', {
-	    text: gettext('Restore'),
-	    disabled: true,
-	    selModel: sm,
-	    enableFn: function(rec) {
-		return !!rec;
-	    },
-	    handler: function(b, e, rec) {
-		let win = Ext.create('PVE.window.Restore', {
-		    nodename: nodename,
-		    vmid: vmid,
-		    volid: rec.data.volid,
-		    volidText: PVE.Utils.render_storage_content(rec.data.volid, {}, rec),
-		    vmtype: vmtype,
-		    isPBS: isPBS,
-		});
-		win.show();
-		win.on('destroy', reload);
-	    },
-	});
-
-	let delete_btn = Ext.create('Proxmox.button.StdRemoveButton', {
-	    selModel: sm,
-	    dangerous: true,
-	    delay: 5,
-	    enableFn: rec => !rec?.data?.protected,
-	    confirmMsg: ({ data }) => {
-		let msg = Ext.String.format(
-		    gettext('Are you sure you want to remove entry {0}'), `'${data.volid}'`);
-		return msg + " " + gettext('This will permanently erase all data.');
-	    },
-	    getUrl: ({ data }) => `/nodes/${nodename}/storage/${storagesel.getValue()}/content/${data.volid}`,
-	    callback: () => reload(),
-	});
-
-	let config_btn = Ext.create('Proxmox.button.Button', {
-	    text: gettext('Show Configuration'),
-	    disabled: true,
-	    selModel: sm,
-	    enableFn: rec => !!rec,
-	    handler: function(b, e, rec) {
-		let storage = storagesel.getValue();
-		if (!storage) {
-		    return;
-		}
-		Ext.create('PVE.window.BackupConfig', {
-		    volume: rec.data.volid,
-		    pveSelNode: me.pveSelNode,
-		    autoShow: true,
-		});
-	    },
-	});
-
-	// declared above so that the storage selector can change this buttons hidden state
-	file_restore_btn = Ext.create('Proxmox.button.Button', {
-	    text: gettext('File Restore'),
-	    disabled: true,
-	    selModel: sm,
-	    enableFn: rec => !!rec && isPBS,
-	    hidden: !isPBS,
-	    handler: function(b, e, rec) {
-		let storage = storagesel.getValue();
-		let isVMArchive = PVE.Utils.volume_is_qemu_backup(rec.data.volid, rec.data.format);
-		Ext.create('Proxmox.window.FileBrowser', {
-		    title: gettext('File Restore') + " - " + rec.data.text,
-		    listURL: `/api2/json/nodes/localhost/storage/${storage}/file-restore/list`,
-		    downloadURL: `/api2/json/nodes/localhost/storage/${storage}/file-restore/download`,
-		    extraParams: {
-			volume: rec.data.volid,
-		    },
-		    archive: isVMArchive ? 'all' : undefined,
-		    autoShow: true,
-		});
-	    },
-	});
-
-	Ext.apply(me, {
-	    selModel: sm,
-	    tbar: {
-		overflowHandler: 'scroller',
-		items: [
-		    backup_btn,
-		    '-',
-		    restore_btn,
-		    file_restore_btn,
-		    config_btn,
-		    {
-			xtype: 'proxmoxButton',
-			text: gettext('Edit Notes'),
-			disabled: true,
-			handler: function() {
-			    let volid = sm.getSelection()[0].data.volid;
-			    var storage = storagesel.getValue();
-			    Ext.create('Proxmox.window.Edit', {
-				autoLoad: true,
-				width: 600,
-				height: 400,
-				resizable: true,
-				title: gettext('Notes'),
-				url: `/api2/extjs/nodes/${nodename}/storage/${storage}/content/${volid}`,
-				layout: 'fit',
-				items: [
-				    {
-					xtype: 'textarea',
-					layout: 'fit',
-					name: 'notes',
-					height: '100%',
-				    },
-				],
-				listeners: {
-				    destroy: () => reload(),
-				},
-			    }).show();
-			},
-		    },
-		    {
-			xtype: 'proxmoxButton',
-			text: gettext('Change Protection'),
-			disabled: true,
-			handler: function(button, event, record) {
-			    let volid = record.data.volid, storage = storagesel.getValue();
-			    let url = `/api2/extjs/nodes/${nodename}/storage/${storage}/content/${volid}`;
-			    let newProtection = record.data.protected ? 0 : 1;
-			    Proxmox.Utils.API2Request({
-				url: url,
-				method: 'PUT',
-				waitMsgTarget: me,
-				params: {
-				    'protected': newProtection,
-				},
-				failure: (response) => Ext.Msg.alert('Error', response.htmlStatus),
-				success: (response) => {
-				    reload();
-				    // propagate to remove button, fake for event as reload is to slow
-				    record.data.protected = newProtection; // TODO: check if writing is OK!
-				    sm.fireEvent('selectionchange', sm, [record]);
-				},
-			    });
-			},
-		    },
-		    '-',
-		    delete_btn,
-		    '->',
-		    storagesel,
-		    '-',
-		    vmidfilterCB,
-		    storagefilter,
-		],
-	    },
-	    columns: [
-		{
-		    header: gettext('Name'),
-		    flex: 2,
-		    sortable: true,
-		    renderer: PVE.Utils.render_storage_content,
-		    dataIndex: 'volid',
-		},
-		{
-		    header: gettext('Notes'),
-		    dataIndex: 'notes',
-		    flex: 1,
-		    renderer: Ext.htmlEncode,
-		},
-		{
-		    header: `<i class="fa fa-shield"></i>`,
-		    tooltip: gettext('Protected'),
-		    width: 30,
-		    renderer: v => v ? `<i data-qtip="${gettext('Protected')}" class="fa fa-shield"></i>` : '',
-		    sorter: (a, b) => (b.data.protected || 0) - (a.data.protected || 0),
-		    dataIndex: 'protected',
-		},
-		{
-		    header: gettext('Date'),
-		    width: 150,
-		    dataIndex: 'vdate',
-		},
-		{
-		    header: gettext('Format'),
-		    width: 100,
-		    dataIndex: 'format',
-		},
-		{
-		    header: gettext('Size'),
-		    width: 100,
-		    renderer: Proxmox.Utils.format_size,
-		    dataIndex: 'size',
-		},
-		{
-		    header: gettext('VMID'),
-		    dataIndex: 'vmid',
-		    hidden: true,
-		},
-		{
-		    header: gettext('Encrypted'),
-		    dataIndex: 'encrypted',
-		    renderer: PVE.Utils.render_backup_encryption,
-		},
-		{
-		    header: gettext('Verify State'),
-		    dataIndex: 'verification',
-		    renderer: PVE.Utils.render_backup_verification,
-		},
-	    ],
-	});
-
-	me.callParent();
-    },
-});
-- 
2.30.2

[pve-devel] [PATCH manager 5/5 v2] Storage ContentView: Remove dead code

[Matthias Heiserer]

These lines were only used by grid/BackupView, which gets deleted in
this series.
 
 
Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
---
 www/manager6/storage/ContentView.js | 22 ++--------------------
 1 file changed, 2 insertions(+), 20 deletions(-)

diff --git a/www/manager6/storage/ContentView.js b/www/manager6/storage/ContentView.js
index 2874b71e..cb22ab4d 100644
--- a/www/manager6/storage/ContentView.js
+++ b/www/manager6/storage/ContentView.js
@@ -155,20 +155,6 @@ Ext.define('PVE.storage.ContentView', {
 		renderer: PVE.Utils.render_storage_content,
 		dataIndex: 'text',
 	    },
-	    'notes': {
-		header: gettext('Notes'),
-		flex: 1,
-		renderer: Ext.htmlEncode,
-		dataIndex: 'notes',
-	    },
-	    'protected': {
-		header: `<i class="fa fa-shield"></i>`,
-		tooltip: gettext('Protected'),
-		width: 30,
-		renderer: v => v ? `<i data-qtip="${gettext('Protected')}" class="fa fa-shield"></i>` : '',
-		sorter: (a, b) => (b.data.protected || 0) - (a.data.protected || 0),
-		dataIndex: 'protected',
-	    },
 	    'date': {
 		header: gettext('Date'),
 		width: 150,
@@ -194,10 +180,6 @@ Ext.define('PVE.storage.ContentView', {
 		delete availableColumns[key];
 	    }
 	});
-
-	if (me.extraColumns && typeof me.extraColumns === 'object') {
-	    Object.assign(availableColumns, me.extraColumns);
-	}
 	const columns = Object.values(availableColumns);
 
 	Ext.apply(me, {
@@ -216,8 +198,8 @@ Ext.define('PVE.storage.ContentView', {
     Ext.define('pve-storage-content', {
 	extend: 'Ext.data.Model',
 	fields: [
-	    'volid', 'content', 'format', 'size', 'used', 'vmid',
-	    'channel', 'id', 'lun', 'notes', 'verification',
+	    'volid', 'content', 'format', 'size', 'used',
+	    'channel', 'id', 'lun',
 	    {
 		name: 'text',
 		convert: function(value, record) {
-- 
2.30.2

Re: [pve-devel] [PATCH manager 2/5 v2] GUI: Utils: Helpers for backup type and icon

[Thomas Lamprecht]

On 18/03/2022 14:52, Matthias Heiserer wrote:
> Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
> ---
> changes from v1:
> add "backup" to name
> return empty string instead of throwing


patch order is wrong though, you cannot use a helper in patch 1/x and only introduce
it in a later patch. This break bisect and causal order, both things I'm fond of :)

order can be fixed on apply for this one, at least if there's nothing else that comes up,
but please avoid that pattern for future series (or if this one gets a v3)

>  www/manager6/Utils.js | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
> index aafe359a..337ccfae 100644
> --- a/www/manager6/Utils.js
> +++ b/www/manager6/Utils.js
> @@ -1803,6 +1803,26 @@ Ext.define('PVE.Utils', {
>  
>  	return undefined;
>      },
> +
> +    get_backup_type_icon_cls: function(volid, format) {
> +	var cls = '';

nit:
we use `let`  for new code, but actually I'd just return directly, like you do in the
other helper, the intermediate variable has no real benefit in this small, straight
forward function.

> +	if (PVE.Utils.volume_is_qemu_backup(volid, format)) {
> +	    cls = 'fa-desktop';
> +	} else if (PVE.Utils.volume_is_lxc_backup(volid, format)) {
> +	    cls = 'fa-cube';
> +	}
> +	return cls;
> +    },
> +
> +    get_backup_type: function(volid, format) {
> +	if (PVE.Utils.volume_is_qemu_backup(volid, format)) {
> +	    return 'qemu';
> +	} else if (PVE.Utils.volume_is_lxc_backup(volid, format)) {
> +	    return 'lxc';
> +	} else {
> +	    return '';
> +	}
> +    },
>  },
>  
>      singleton: true,

Re: [pve-devel] [PATCH manager 1/5 v2] Storage GUI: Rewrite backup content view as TreePanel.

[Fabian Ebner]

Nit: for the commit title, prefixing like "ui: storage:" is preferred.

I really think we should start with the tree fully expanded, or we will
get shouted at by users relying on notes to find their backups ;)

@Thomas: The backups are now ordered ascending by date again. In PBS
it's also like that. Should that be changed or do we switch back in PVE?

Am 18.03.22 um 14:52 schrieb Matthias Heiserer:
> +Ext.define('PVE.storage.BackupView', {
> +    extend: 'Ext.tree.Panel',
>      alias: 'widget.pveStorageBackupView',
> +    mixins: ['Proxmox.Mixin.CBind'],
> +    rootVisible: false,
>  
> -    showColumns: ['name', 'notes', 'protected', 'date', 'format', 'size'],
> +    title: gettext('Content'),
>  
> -    initComponent: function() {
> -	var me = this;
> +    cbindData: function(initialCfg) {
> +	this.isPBS = initialCfg.pluginType === 'pbs';

That's part of what cbind should do for you if you return { isPBS: ...
}, and that's how it should be used to avoid breaking the abstraction.

But since the storage can change (in the guest backup view) this should
rather be a normal bind and be updated there.

> +	return {};
> +    },
> +    isStorage: false,

Doesn't seem to be used.

>  
> -	var nodename = me.nodename = me.pveSelNode.data.node;
> -	if (!nodename) {
> -	    throw "no node name specified";
> -	}
> +    controller: {
> +	xclass: 'Ext.app.ViewController',
>  
> -	var storage = me.storage = me.pveSelNode.data.storage;
> -	if (!storage) {
> -	    throw "no storage ID specified";
> -	}
> +	groupnameHelper: function(item) {

Nit: IMHO something like getGroupName() would be a bit more readable at
the call sites.

> +	    if (item.vmid) {
> +		return PVE.Utils.get_backup_type(item.volid, item.format) + `/${item.vmid}`;
> +	    } else {
> +		return 'Other';
> +	    }
> +	},
>  
> -	me.content = 'backup';
> +	init: function(view) {
> +	    let me = this;
> +	    me.storage = view?.pveSelNode?.data?.storage;

Nit: here you use ?., but below you assume it's set (which I suppose we
can).

> +	    me.nodename = view.nodename || view.pveSelNode.data.node;

Is there ever a case where it's not the selected node?

> +	    me.vmid = view.pveSelNode.data.vmid;
> +	    me.vmtype = view.pveSelNode.data.type;
>  
> -	var sm = me.sm = Ext.create('Ext.selection.RowModel', {});
> +	    me.store = Ext.create('Ext.data.Store', {
> +		model: 'PVE.storage.BackupModel',
> +	    });
> +	    me.store.on('load', me.onLoad, me);
> +	    view.getStore().setConfig('filterer', 'bottomup');
> +	    view.getStore().setSorters(['text']);
>  
> -	var reload = function() {
> -	    me.store.load();
> -	};
> +	    if (me.vmid) {
> +		me.getView().getStore().filter({
> +		    property: 'vmid',
> +		    value: me.vmid,
> +		    exactMatch: true,
> +		});
> +	    } else {
> +		me.lookup('storagesel').setVisible(false);
> +		me.lookup('backupNowButton').setVisible(false);
> +	    }
> +	    Proxmox.Utils.monStoreErrors(view, me.store);
> +	},
>  
> -	let pruneButton = Ext.create('Proxmox.button.Button', {
> -	    text: gettext('Prune group'),
> -	    disabled: true,
> -	    selModel: sm,
> -	    setBackupGroup: function(backup) {
> -		if (backup) {
> -		    let name = backup.text;
> -		    let vmid = backup.vmid;
> -		    let format = backup.format;
> +	onLoad: function(store, records, success, operation) {
> +	    let me = this;
> +	    let view = me.getView();
> +	    let selection = view.getSelection()?.[0];
> +	    selection = selection?.parentNode?.data?.text +selection?.data?.volid;

Style nit: missing space after + and could use `${...}${...}` syntax
instead.

(...)

> +	    if (selection) {
> +		let rootnode = view.getRootNode();
> +		let selected;
> +		rootnode.cascade(node => {
> +		    if (selected) {return false;} // skip if already found

Style nit: if body on the same line

> +		    let id = node.parentNode?.data?.text + node.data?.volid;
> +		    if (id === selection) {
> +			selected = node;
> +			return false;
> +		    }
> +		    return true;
>  		});
> -		win.show();
> -		win.on('destroy', reload);
> -	    },
> -	});
> +		view.setSelection(selected);
> +		view.getView().focusRow(selected);

After removing a backup I get a
  Uncaught TypeError: node is undefined
referencing this line.

> +	    }
> +	    Proxmox.Utils.setErrorMask(view, false);
> +	},
>  

(...)

> +
> +	removeHandler: function(button, event, rec) {
> +	    let me = this;
> +	    const volid = rec.data.volid;
> +	    Proxmox.Utils.API2Request({
> +		url: `/nodes/${me.nodename}/storage/${me.storage}/content//${volid}`,

Nit: duplicate / in line above

> +		method: 'DELETE',
> +		callback: () => me.reload(),
> +		failure: response => Ext.Msg.alert(gettext('Error'), response.htmlStatus),
> +	    });
> +	},
> +
> +	searchKeyupFn: function(field) {
> +	    let me = this;
> +	    me.getView().getStore().getFilters().removeByKey('volid');
> +	    me.getView().getStore().filter([
> +		{
> +		    property: 'volid',
> +		    value: field.getValue(),
> +		    anyMatch: true,
> +		    caseSensitive: false,
> +		    id: 'volid',
>  		},
> -		verification: {
> -		    header: gettext('Verify State'),
> -		    dataIndex: 'verification',
> -		    renderer: PVE.Utils.render_backup_verification,
> +	    ]);
> +	},
> +
> +	searchClearHandler: function(field) {
> +	    field.triggers.clear.setVisible(false);
> +	    field.setValue(this.originalValue);
> +	    this.getView().getStore().clearFilter();
> +	},
> +
> +	searchChangeFn: function(field, newValue, oldValue) {
> +	    if (newValue !== field.originalValue) {
> +		field.triggers.clear.setVisible(true);
> +	    }
> +	},
> +
> +	storageSelectorBoxReady: function(selector, width, height, eOpts) {
> +	    selector.setNodename(this.nodename);
> +	},

Would cbind also be an option?

> +
> +	storageSelectorChange: function(self, newValue, oldValue, eOpts) {
> +	    let me = this;
> +	    me.storage = newValue;
> +	    me.getView().getSelectionModel().deselectAll();
> +	    me.reload();
> +	},
> +
> +	backupNowHandler: function(button, event) {
> +	    let me = this;
> +	    Ext.create('PVE.window.Backup', {
> +		nodename: me.nodename,
> +		vmid: me.vmid,
> +		vmtype: me.vmtype,
> +		storage: me.storage,
> +		listeners: {
> +		    close: () => me.reload(),
>  		},
> -	    };
> -	}
> +	    }).show();
> +	},
> +    },
> +
> +    columns: [
> +	{
> +	    xtype: 'treecolumn',
> +	    header: gettext("Backup Group"),
> +	    dataIndex: 'text',
> +	    renderer: function(value, _metadata, record) {
> +		if (record.phantom) { return value; }

Style nit: if body on the same line. Could be avoided using ternary ? here.

> +		return PVE.Utils.render_storage_content(...arguments);
> +	    },
> +	    flex: 2,
> +	},
> +	{
> +	    header: gettext('Notes'),
> +	    flex: 1,
> +	    renderer: Ext.htmlEncode,
> +	    dataIndex: 'notes',
> +	},
> +	{
> +	    header: `<i class="fa fa-shield"></i>`,
> +	    tooltip: gettext('Protected'),
> +	    width: 30,
> +	    renderer: v => v ? `<i data-qtip="${gettext('Protected')}" class="fa fa-shield"></i>` : '',

Style nit: line too long

> +	    sorter: (a, b) => (b.data.protected || 0) - (a.data.protected || 0),
> +	    dataIndex: 'protected',
> +	},
> +	{
> +	    header: gettext('Date'),
> +	    width: 150,
> +	    dataIndex: 'ctime',
> +	    xtype: 'datecolumn',
> +	    format: 'Y-m-d H:i:s',
> +	},
> +	{
> +	    header: gettext('Format'),
> +	    width: 100,
> +	    dataIndex: 'format',
> +	},
> +	{
> +	    header: gettext('Size'),
> +	    width: 100,
> +	    renderer: Proxmox.Utils.format_size,
> +	    dataIndex: 'size',
> +	},
> +	{
> +	    header: gettext('Encrypted'),
> +	    dataIndex: 'encrypted',
> +	    renderer: PVE.Utils.render_backup_encryption,
> +	    cbind: {
> +		hidden: '{!isPBS}',
> +	    },
> +	},
> +	{
> +	    header: gettext('Verify State'),
> +	    dataIndex: 'verification',
> +	    renderer: PVE.Utils.render_backup_verification,
> +	    cbind: {
> +		hidden: '{!isPBS}',
> +	    },
> +	},
> +    ],
> +
> +    tbar: [
> +	{
> +	    xtype: 'button',
> +	    text: gettext('Backup now'),
> +	    handler: 'backupNowHandler',
> +	    reference: 'backupNowButton',
> +	},
> +	{
> +	    xtype: 'proxmoxButton',
> +	    text: gettext('Restore'),
> +	    handler: 'restoreHandler',
> +	    parentXType: "treepanel",
> +	    disabled: true,
> +	    enableFn: record => record.phantom === false,

Style nit: !record.phantom is shorter

Repeated below a few times

> +	},
> +	{
> +	    xtype: 'proxmoxButton',
> +	    text: gettext('File Restore'),
> +	    handler: 'restoreFilesHandler',
> +	    cbind: {
> +		hidden: '{!isPBS}',
> +	    },
> +	    parentXType: "treepanel",
> +	    disabled: true,
> +	    enableFn: record => record.phantom === false,
> +	},
> +	{
> +	    xtype: 'proxmoxButton',
> +	    text: gettext('Show Configuration'),
> +	    handler: 'showConfigurationHandler',
> +	    parentXType: "treepanel",
> +	    disabled: true,
> +	    enableFn: record => record.phantom === false,
> +	},
> +	{
> +	    xtype: 'proxmoxButton',
> +	    text: gettext('Edit Notes'),
> +	    handler: 'editNotesHandler',
> +	    parentXType: "treepanel",
> +	    disabled: true,
> +	    enableFn: record => record.phantom === false,
> +	},
> +	{
> +	    xtype: 'proxmoxButton',
> +	    text: gettext('Change Protection'),
> +	    handler: 'changeProtectionHandler',
> +	    parentXType: "treepanel",
> +	    disabled: true,
> +	    enableFn: record => record.phantom === false,
> +	},
> +	'-',
> +	{
> +	    xtype: 'proxmoxButton',
> +	    text: gettext('Prune group'),
> +	    setBackupGroup: function(backup) {
> +		let me = this;
> +		if (backup) {
> +		    let volid = backup.volid;
> +		    let vmid = backup.vmid;
> +		    let format = backup.format;
>  
> -	me.callParent();
> +		    let vmtype = PVE.Utils.get_backup_type(volid, format);
> +		    if (vmid && vmtype) {
> +			me.setText(gettext('Prune group') + ` ${vmtype}/${vmid}`);
> +			me.vmid = vmid;
> +			me.vmtype = vmtype;
> +			me.setDisabled(false);
> +			return;
> +		    }
> +		}
> +		me.setText(gettext('Prune group'));
> +		me.vmid = null;
> +		me.vmtype = null;
> +		me.setDisabled(true);
> +	    },
> +	    handler: 'pruneGroupHandler',
> +	    parentXType: "treepanel",
> +	    disabled: true,
> +	    reference: 'pruneButton',
> +	    enableFn: () => true,

This makes the disabling in setBackupGroup not work as intended.

> +	},
> +	{
> +	    xtype: 'proxmoxButton',
> +	    text: gettext('Remove'),
> +	    handler: 'removeHandler',
> +	    parentXType: 'treepanel',
> +	    disabled: true,
> +	    enableFn: record => record.phantom === false && !record?.data?.protected,
> +	    confirmMsg: function(rec) {
> +		let name = rec.data.text;
> +		return Ext.String.format(gettext('Are you sure you want to remove entry {0}'), `'${name}'`);

Sytle nit: line too long

> +	    },
> +	},
> +	'->',
> +	{
> +	    xtype: 'pveStorageSelector',
> +	    fieldLabel: gettext('Storage'),
> +	    storageContent: 'backup',
> +	    reference: 'storagesel',
> +	    listeners: {
> +		change: 'storageSelectorChange',
> +		boxready: 'storageSelectorBoxReady',
> +	    },
>  
> -	me.store.getSorters().clear();
> -	me.store.setSorters([
> -	    {
> -		property: 'vmid',
> -		direction: 'ASC',
> +	},
> +	gettext('Search') + ':',
> +	' ',
> +	{
> +	    xtype: 'textfield',
> +	    width: 200,
> +	    enableKeyEvents: true,
> +	    emptyText: gettext('Name, Format'),
> +	    listeners: {
> +		keyup: {
> +		    buffer: 500,
> +		    fn: 'searchKeyupFn',
> +		},
> +		change: 'searchChangeFn',
>  	    },
> -	    {
> -		property: 'vdate',
> -		direction: 'DESC',
> +	    triggers: {
> +		clear: {
> +		    cls: 'pmx-clear-trigger',
> +		    weight: -1,
> +		    hidden: true,
> +		    handler: 'searchClearHandler',
> +		},
>  	    },
> -	]);
> +	},
> +	],
> +
> +    listeners: {
> +	activate: function() {
> +	    let me = this;
> +	    // only load on first activate to not load every tab switch
> +	    if (!me.firstLoad) {
> +		me.getController().reload();
> +		me.firstLoad = true;
> +	    }

This does not seem to apply here, because there are no tabs to switch
to/from. Switching to and back from a different storage content type
leads to me.firstLoad being undefined again. But I'd actually want to
see changes when doing so in any case ;)

> +	},
> +	selectionchange: function(model, srecords, eOpts) {
> +	    let pruneButton = this.getController().lookup('pruneButton');
> +	    if (srecords.length === 1) {
> +		pruneButton.setBackupGroup(srecords[0].data);
> +	    } else {
> +		pruneButton.setBackupGroup(null);
> +	    }
> +	},
>      },
>  });

Re: [pve-devel] [PATCH manager 3/5 v2] Backup GUI: Use the new storage/BackupView instead of grid/BackupView.

[Fabian Ebner]

There's certain things that need to be changed to not break existing
work flows:

* Restoring doesn't overwrite the existing guest anymore and can't be
used for that anymore.
* Should not only filter by ID, but by type + ID.
* Cannot get rid of the ID filtering anymore. Currently, it always
filters by type, so we might want to keep that behavior.

Nit: introducing the guest-view specific functionality to the BackupView
class could've been part of this patch (or its own preparatory one), but
not sure if it's worth the effort (anymore).

Am 18.03.22 um 14:52 schrieb Matthias Heiserer:
> Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
> ---
>  www/manager6/lxc/Config.js  | 2 +-
>  www/manager6/qemu/Config.js | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/www/manager6/lxc/Config.js b/www/manager6/lxc/Config.js
> index 89b59c9b..242780c8 100644
> --- a/www/manager6/lxc/Config.js
> +++ b/www/manager6/lxc/Config.js
> @@ -256,7 +256,7 @@ Ext.define('PVE.lxc.Config', {
>  	    me.items.push({
>  		title: gettext('Backup'),
>  		iconCls: 'fa fa-floppy-o',
> -		xtype: 'pveBackupView',
> +		xtype: 'pveStorageBackupView',
>  		itemId: 'backup',
>  	    },
>  	    {
> diff --git a/www/manager6/qemu/Config.js b/www/manager6/qemu/Config.js
> index 9fe933df..3ed2427a 100644
> --- a/www/manager6/qemu/Config.js
> +++ b/www/manager6/qemu/Config.js
> @@ -291,7 +291,7 @@ Ext.define('PVE.qemu.Config', {
>  	    me.items.push({
>  		title: gettext('Backup'),
>  		iconCls: 'fa fa-floppy-o',
> -		xtype: 'pveBackupView',
> +		xtype: 'pveStorageBackupView',
>  		itemId: 'backup',
>  	    },
>  	    {

Re: [pve-devel] [PATCH manager 4/5 v2] Remove grid/backupview as it got replaced by storage/backupview

[Fabian Ebner]

Am 18.03.22 um 14:52 schrieb Matthias Heiserer:
> Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
> ---
>  www/manager6/grid/BackupView.js | 388 --------------------------------
>  1 file changed, 388 deletions(-)
>  delete mode 100644 www/manager6/grid/BackupView.js
> 

Also needs to remove it from the Makefile or the package won't build
anymore.

Re: [pve-devel] [PATCH manager 5/5 v2] Storage ContentView: Remove dead code

[Fabian Ebner]

Am 18.03.22 um 14:52 schrieb Matthias Heiserer:
> These lines were only used by grid/BackupView, which gets deleted in
> this series.
>  
>  
> Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
> ---
>  www/manager6/storage/ContentView.js | 22 ++--------------------
>  1 file changed, 2 insertions(+), 20 deletions(-)
> 
> diff --git a/www/manager6/storage/ContentView.js b/www/manager6/storage/ContentView.js
> index 2874b71e..cb22ab4d 100644
> --- a/www/manager6/storage/ContentView.js
> +++ b/www/manager6/storage/ContentView.js
> @@ -155,20 +155,6 @@ Ext.define('PVE.storage.ContentView', {
>  		renderer: PVE.Utils.render_storage_content,
>  		dataIndex: 'text',
>  	    },
> -	    'notes': {
> -		header: gettext('Notes'),
> -		flex: 1,
> -		renderer: Ext.htmlEncode,
> -		dataIndex: 'notes',
> -	    },
> -	    'protected': {
> -		header: `<i class="fa fa-shield"></i>`,
> -		tooltip: gettext('Protected'),
> -		width: 30,
> -		renderer: v => v ? `<i data-qtip="${gettext('Protected')}" class="fa fa-shield"></i>` : '',
> -		sorter: (a, b) => (b.data.protected || 0) - (a.data.protected || 0),
> -		dataIndex: 'protected',
> -	    },
>  	    'date': {
>  		header: gettext('Date'),
>  		width: 150,
> @@ -194,10 +180,6 @@ Ext.define('PVE.storage.ContentView', {
>  		delete availableColumns[key];
>  	    }

From a quick look, the showColumns special casing right above here seems
to also be unnecessary now.

>  	});
> -
> -	if (me.extraColumns && typeof me.extraColumns === 'object') {
> -	    Object.assign(availableColumns, me.extraColumns);
> -	}
>  	const columns = Object.values(availableColumns);
>  
>  	Ext.apply(me, {
> @@ -216,8 +198,8 @@ Ext.define('PVE.storage.ContentView', {
>      Ext.define('pve-storage-content', {
>  	extend: 'Ext.data.Model',
>  	fields: [
> -	    'volid', 'content', 'format', 'size', 'used', 'vmid',
> -	    'channel', 'id', 'lun', 'notes', 'verification',
> +	    'volid', 'content', 'format', 'size', 'used',
> +	    'channel', 'id', 'lun',
>  	    {
>  		name: 'text',
>  		convert: function(value, record) {

Re: [pve-devel] [PATCH manager 1/5 v2] Storage GUI: Rewrite backup content view as TreePanel.

[Thomas Lamprecht]

On 22.03.22 09:42, Fabian Ebner wrote:
> Nit: for the commit title, prefixing like "ui: storage:" is preferred.
> 
> I really think we should start with the tree fully expanded, or we will
> get shouted at by users relying on notes to find their backups ;)

+1, additionally add a toggle for expand/collapse-all for quickly switching
but default should be all visible.

> 
> @Thomas: The backups are now ordered ascending by date again. In PBS
> it's also like that. Should that be changed or do we switch back in PVE?

As age vs. date order is exactly the reverse, and thus possibly confusing:
I'd like have the most recent first (at top).

Re: [pve-devel] [PATCH manager 1/5 v2] Storage GUI: Rewrite backup content view as TreePanel.

[Matthias Heiserer]

8<

>> +	onLoad: function(store, records, success, operation) {
>> +	    let me = this;
>> +	    let view = me.getView();
>> +	    let selection = view.getSelection()?.[0];
>> +	    selection = selection?.parentNode?.data?.text +selection?.data?.volid;
> 
> Style nit: missing space after + and could use `${...}${...}` syntax
> instead.
That syntax won't work because then, if both parameters are undefined, 
the result would be a string instead of a falsy (NaN) value.
There's probably a better way of doing this.

> 
> (...)
> 
>> +	    if (selection) {
>> +		let rootnode = view.getRootNode();
>> +		let selected;
>> +		rootnode.cascade(node => {
>> +		    if (selected) {return false;} // skip if already found
> 
> Style nit: if body on the same line
Is 'if on one line' something we generally don't do? It appears 
occasionally in the code.
> 
>> +		method: 'DELETE',
>> +		callback: () => me.reload(),
>> +		failure: response => Ext.Msg.alert(gettext('Error'), response.htmlStatus),
>> +	    });
>> +	},
>> +
>> +	searchKeyupFn: function(field) {
>> +	    let me = this;
>> +	    me.getView().getStore().getFilters().removeByKey('volid');
>> +	    me.getView().getStore().filter([
>> +		{
>> +		    property: 'volid',
>> +		    value: field.getValue(),
>> +		    anyMatch: true,
>> +		    caseSensitive: false,
>> +		    id: 'volid',
>>   		},
>> -		verification: {
>> -		    header: gettext('Verify State'),
>> -		    dataIndex: 'verification',
>> -		    renderer: PVE.Utils.render_backup_verification,
>> +	    ]);
>> +	},
>> +
>> +	searchClearHandler: function(field) {
>> +	    field.triggers.clear.setVisible(false);
>> +	    field.setValue(this.originalValue);
>> +	    this.getView().getStore().clearFilter();
>> +	},
>> +
>> +	searchChangeFn: function(field, newValue, oldValue) {
>> +	    if (newValue !== field.originalValue) {
>> +		field.triggers.clear.setVisible(true);
>> +	    }
>> +	},
>> +
>> +	storageSelectorBoxReady: function(selector, width, height, eOpts) {
>> +	    selector.setNodename(this.nodename);
>> +	},
> 
> Would cbind also be an option?
> 
Don't think so, as nodename comes from pveSelNode.
However, normal bind works :)

Re: [pve-devel] [PATCH manager 1/5 v2] Storage GUI: Rewrite backup content view as TreePanel.

[Fabian Ebner]

Am 30.03.22 um 14:59 schrieb Matthias Heiserer:
> 8<
> 
>>> +    onLoad: function(store, records, success, operation) {
>>> +        let me = this;
>>> +        let view = me.getView();
>>> +        let selection = view.getSelection()?.[0];
>>> +        selection = selection?.parentNode?.data?.text
>>> +selection?.data?.volid;
>>
>> Style nit: missing space after + and could use `${...}${...}` syntax
>> instead.
> That syntax won't work because then, if both parameters are undefined,
> the result would be a string instead of a falsy (NaN) value.
> There's probably a better way of doing this.
> 

Ok.

>>
>> (...)
>>
>>> +        if (selection) {
>>> +        let rootnode = view.getRootNode();
>>> +        let selected;
>>> +        rootnode.cascade(node => {
>>> +            if (selected) {return false;} // skip if already found
>>
>> Style nit: if body on the same line
> Is 'if on one line' something we generally don't do? It appears
> occasionally in the code.

I don't think it's explicitly forbidden by our style guide, which
essentially is what the linter complains about, but in the (vast)
majority of cases, the body is on its own line.

>>
>>> +        method: 'DELETE',
>>> +        callback: () => me.reload(),
>>> +        failure: response => Ext.Msg.alert(gettext('Error'),
>>> response.htmlStatus),
>>> +        });
>>> +    },
>>> +
>>> +    searchKeyupFn: function(field) {
>>> +        let me = this;
>>> +        me.getView().getStore().getFilters().removeByKey('volid');
>>> +        me.getView().getStore().filter([
>>> +        {
>>> +            property: 'volid',
>>> +            value: field.getValue(),
>>> +            anyMatch: true,
>>> +            caseSensitive: false,
>>> +            id: 'volid',
>>>           },
>>> -        verification: {
>>> -            header: gettext('Verify State'),
>>> -            dataIndex: 'verification',
>>> -            renderer: PVE.Utils.render_backup_verification,
>>> +        ]);
>>> +    },
>>> +
>>> +    searchClearHandler: function(field) {
>>> +        field.triggers.clear.setVisible(false);
>>> +        field.setValue(this.originalValue);
>>> +        this.getView().getStore().clearFilter();
>>> +    },
>>> +
>>> +    searchChangeFn: function(field, newValue, oldValue) {
>>> +        if (newValue !== field.originalValue) {
>>> +        field.triggers.clear.setVisible(true);
>>> +        }
>>> +    },
>>> +
>>> +    storageSelectorBoxReady: function(selector, width, height, eOpts) {
>>> +        selector.setNodename(this.nodename);
>>> +    },
>>
>> Would cbind also be an option?
>>
> Don't think so, as nodename comes from pveSelNode.
> However, normal bind works :)

I mean, you would need to return it as part of the cbindData() ;)

Re: [pve-devel] [PATCH manager 1/5 v2] Storage GUI: Rewrite backup content view as TreePanel.

[Thomas Lamprecht]

On 31.03.22 08:36, Fabian Ebner wrote:
> Am 30.03.22 um 14:59 schrieb Matthias Heiserer:
>> 8<
>>
>>>> +    onLoad: function(store, records, success, operation) {
>>>> +        let me = this;
>>>> +        let view = me.getView();
>>>> +        let selection = view.getSelection()?.[0];
>>>> +        selection = selection?.parentNode?.data?.text
>>>> +selection?.data?.volid;
>>> Style nit: missing space after + and could use `${...}${...}` syntax
>>> instead.
>> That syntax won't work because then, if both parameters are undefined,
>> the result would be a string instead of a falsy (NaN) value.
>> There's probably a better way of doing this.
>>

seems odd use of side-effect, if that's relevant it could be checked more explicitly
with a boolean expression (as JS "arithmetic" operations can be quite weird).

Note also that `var foo = "asdf" + undefined;` makes `foo` be "asdfundefined" so
are you sure that's the behavior you want?

FWIW, you could also destructure an additional object level like:

let { data, parentNode } = view.getSelection()?.[0] ?? {};
 
Could make checking explicitly easier.


>>> (...)
>>>
>>>> +        if (selection) {
>>>> +        let rootnode = view.getRootNode();
>>>> +        let selected;
>>>> +        rootnode.cascade(node => {
>>>> +            if (selected) {return false;} // skip if already found
>>> Style nit: if body on the same line
>> Is 'if on one line' something we generally don't do? It appears
>> occasionally in the code.
> I don't think it's explicitly forbidden by our style guide, which
> essentially is what the linter complains about, but in the (vast)
> majority of cases, the body is on its own line.
> 

Thanks for the hint, it's now:
https://pve.proxmox.com/wiki/Javascript_Style_Guide#Single-Line_If-Statement