[pve-devel] [PATCH v4 manager 0/4] BackupView as TreePanel

[pve-devel] [PATCH v4 manager 0/4] BackupView as TreePanel

[Matthias Heiserer]

Depends on https://lists.proxmox.com/pipermail/pve-devel/2022-March/052322.html

Matthias Heiserer (4):
  ui: Utils: Helpers for backup type and icon
  ui: storage: Rewrite backup content view as TreePanel.
  ui: delete BackupView and replace it with the new Tree BackupView
  ui: content view: remove dead code

 www/manager6/Makefile               |   1 -
 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  | 817 +++++++++++++++++++++-------
 www/manager6/storage/ContentView.js |  43 +-
 7 files changed, 657 insertions(+), 616 deletions(-)
 delete mode 100644 www/manager6/grid/BackupView.js

-- 
2.30.2

[pve-devel] [PATCH v4 manager 1/4] ui: Utils: Helpers for backup type and icon

[Matthias Heiserer]

Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
---
no changes from v3.

changes from v2:
return in else branch

changes from v1:
add "backup" to name

 www/manager6/Utils.js | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 735806aa..8c76cdb2 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1800,6 +1800,26 @@ Ext.define('PVE.Utils', {
 
 	return undefined;
     },
+
+    get_backup_type_icon_cls: function(volid, format) {
+	if (PVE.Utils.volume_is_qemu_backup(volid, format)) {
+	    return 'fa-desktop';
+	} else if (PVE.Utils.volume_is_lxc_backup(volid, format)) {
+	    return 'fa-cube';
+	} else {
+	    return '';
+	}
+    },
+
+    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 v4 manager 2/4] ui: storage: 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 v3:
when guest backup is PBS, show encrypted and verify state columns
 and the "file restore" button
BackupView: when searching, search in both format and volid

changes from v2:
Backup GUI: Use the new storage/BackupView instead of grid/BackupView.
expand/collapse button
allow restoring into current guest
when clearing search field, only remove volid filter
add "filter VMID" button, reorder tbar buttons
filter vmid/type button
storage less wide label
sort first by backup group, then by vmid
allow multisort
rename groupnameHelper to getGroupName
assume pveSelNode is set
code cleanup: missing space, break long line
remove duplicate /
use AltTextButton
use normal bind instead of cbind
don't error on missing selection, e.g. after removal
simplify `record-phantom === false` to `!record.phantom`
disable pruning in 'Other' group
use ternary instead of two ifs
remove unnecessary code in activate listener
use bind instead of boxready listener to set nodename of storageselector
less horrific way of setting selection to falsy value; concat it using string template syntax

Changes from v1:
also replace grid/BackupView with storage/BackupView
rename notPBS to PBS, make it available in the view
Display combined size of backups for groups, except for PBS where
deduplication happens
remove timeout for API call
liverestore iff PBS
use get_backup_type helper + fix a bug where 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.
For unknown/renamed backups, remove the exception in get_backup_type
and group them in the folder "Other"
cleanup: reorder statements, remove one-character variable, remove debug log, rename this to me
add text field to model. fix date field in model
remember expanded nodes and selection
rename backup model
use filterer only with correct store
add BackupNow button to create backups

 www/manager6/storage/BackupView.js | 817 ++++++++++++++++++++++-------
 1 file changed, 627 insertions(+), 190 deletions(-)

diff --git a/www/manager6/storage/BackupView.js b/www/manager6/storage/BackupView.js
index 2328c0fc..b7b5df5b 100644
--- a/www/manager6/storage/BackupView.js
+++ b/www/manager6/storage/BackupView.js
@@ -1,223 +1,660 @@
-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',
 
-    showColumns: ['name', 'notes', 'protected', 'date', 'format', 'size'],
+    rootVisible: false,
+    multiColumnSort: true,
+
+    title: gettext('Content'),
+
+    viewModel: {
+	data: {
+	    isPBS: false,
+	    isStorage: true,
+	},
+    },
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
 
-    initComponent: function() {
-	var me = this;
+	collapseAllStatus: true,
 
-	var nodename = me.nodename = me.pveSelNode.data.node;
-	if (!nodename) {
-	    throw "no node name specified";
-	}
+	getGroupName: function(item) {
+	    if (item.vmid) {
+		return PVE.Utils.get_backup_type(item.volid, item.format) + `/${item.vmid}`;
+	    } else {
+		return 'Other';
+	    }
+	},
+
+	guestFilter: function() {
+	    let me = this;
+	    return [
+		{
+		    property: 'vmid',
+		    id: 'vmid',
+		    value: me.vmid,
+		    exactMatch: true,
+		},
+		{
+		    property: 'backupType',
+		    id: 'backupType',
+		    value: me.backupType,
+		    exactMatch: true,
+		},
+	    ];
+	},
 
-	var storage = me.storage = me.pveSelNode.data.storage;
-	if (!storage) {
-	    throw "no storage ID specified";
-	}
+	init: function(view) {
+	    let me = this;
+	    me.storage = view.pveSelNode.data.storage;
+	    me.nodename = view.pveSelNode.data.node;
+	    me.vmid = view.pveSelNode.data.vmid;
+	    me.backupType = view.pveSelNode.data.type;
+	    me.vmtype = view.pveSelNode.data.type;
 
-	me.content = 'backup';
+	    me.store = Ext.create('Ext.data.Store', {
+		model: 'PVE.storage.BackupModel',
+	    });
+	    me.store.on('load', me.onLoad, me);
+	    // start with all groups expanded
+	    me.store.on('load', () => me.lookup('collapseToggle').click(), me, { single: true });
+	    view.getStore().setConfig('filterer', 'bottomup');
+	    view.getStore().setSorters([
+		{
+		    property: 'groupName',
+		    direction: 'ASC',
+		},
+		{
+		    property: 'ctime',
+		    direction: 'DESC',
+		},
+	    ]);
 
-	var sm = me.sm = Ext.create('Ext.selection.RowModel', {});
+	    let viewModel = me.getViewModel();
+	    viewModel.set('nodename', me.nodename);
+	    viewModel.set('isPBS', view.pluginType === 'pbs');
+
+	    if (me.vmid) {
+		me.getView().getStore().filter(me.guestFilter());
+		viewModel.set('isStorage', false);
+	    } else {
+		me.lookup('storagesel').setVisible(false);
+		me.lookup('backupNowButton').setVisible(false);
+	    }
+	    Proxmox.Utils.monStoreErrors(view, me.store);
+	},
+
+	onLoad: function(store, records, success, operation) {
+	    let me = this;
+	    let view = me.getView();
+	    let selection = view.getSelection()?.[0];
+	    selection = selection
+		? `${selection.parentNode.data.text}${selection.data.volid}`
+		: false;
+
+	    let storage = PVE.data.ResourceStore.findRecord(
+		'id',
+		`storage/${me.nodename}/${me.storage}`,
+		0, // startIndex
+		false, // anyMatch
+		true, // caseSensitive
+		true, // exactMatch
+	    );
+	    let viewModel = me.getViewModel();
+	    viewModel.set('isPBS', storage.data.plugintype === 'pbs');
+
+	    let expanded = {};
+	    view.getRootNode().cascadeBy({
+		before: item => {
+		    if (item.isExpanded() && !item.data.leaf) {
+			let id = item.data.text;
+			expanded[me.storage + '/' + id] = true;
+			return true;
+		    }
+		    return false;
+		},
+		after: Ext.emptyFn,
+	    });
+	    let groups = me.getRecordGroups(records, expanded);
+
+	    for (const item of records.map(i => i.data)) {
+		item.text = item.volid;
+		item.leaf = true;
+		item.iconCls = 'fa-file-o';
+		item.backupType = PVE.Utils.get_backup_type(item.volid, item.format);
+		item.groupName = me.getGroupName(item);
+		groups[me.getGroupName(item)].children.push(item);
+		groups[me.getGroupName(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 (viewModel.get('isPBS')) {
+		    group.size = latest.size;
+		}
+		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;
+		});
+		if (selected) {
+		    view.setSelection(selected);
+		    view.getView().focusRow(selected);
+		} else {
+		    me.getView().getSelectionModel().deselectAll();
+		}
+	    }
+	    Proxmox.Utils.setErrorMask(view, false);
+	},
+
+	reload: function() {
+	    let me = this;
+	    let view = me.getView();
+	    if (!view.store || !me.store) {
+		console.warn('cannot reload, no store(s)');
+		return;
+	    }
+
+	    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',
+		},
+	    });
 
-	var reload = function() {
 	    me.store.load();
-	};
+	    Proxmox.Utils.monStoreErrors(view, me.store);
+	},
 
-	let pruneButton = Ext.create('Proxmox.button.Button', {
-	    text: gettext('Prune group'),
+	getRecordGroups: function(records, expanded) {
+	    let groups = {};
+	    let me = this;
+	    const storage = me.storage;
+	    for (const item of records) {
+		const groupName = me.getGroupName(item.data);
+		groups[groupName] = {
+		    vmid: item.data.vmid,
+		    leaf: false,
+		    children: [],
+		    expanded: !!expanded[storage + '/' + groupName],
+		    text: groupName,
+		    ctime: 0,
+		    groupName,
+		    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,
+		vmid: me.vmid,
+		isPBS: me.getViewModel().get('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,
+		},
+		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',
+			name: 'notes',
+			height: '100%',
+		    },
+		],
+		listeners: {
+		    destroy: () => me.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(),
+		},
+	    }).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('searchFilter');
+	    let volidFilter = Ext.create('Ext.util.Filter', {
+		property: 'volid',
+		value: field.getValue(),
+		anyMatch: true,
+		caseSensitive: false,
+	    });
+	    let formatFilter = Ext.create('Ext.util.Filter', {
+		property: 'format',
+		value: field.getValue(),
+		anyMatch: true,
+		caseSensitive: false,
+	    });
+	    me.getView().getStore().filter({
+		filterFn: item => volidFilter.filter(item.data) ||
+		    formatFilter.filter(item.data),
+		id: 'searchFilter',
+	    });
+	},
+
+	searchClearHandler: function(field) {
+	    field.triggers.clear.setVisible(false);
+	    field.setValue(this.originalValue);
+	    this.getView().getStore().getFilters().removeByKey('searchFilter');
+	},
+
+	searchChangeFn: function(field, newValue, oldValue) {
+	    if (newValue !== field.originalValue) {
+		field.triggers.clear.setVisible(true);
+	    }
+	},
+
+	storageSelectorChange: function(self, newValue, oldValue, eOpts) {
+	    let me = this;
+	    if (!me.getViewModel().get('isStorage')) {
+		me.storage = newValue;
+		me.collapseAllStatus = true;
+		me.store.on('load', () => me.lookup('collapseToggle').click(), me, { single: true });
+		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();
+	},
+
+	toggleCollapseHandler: function(button, event) {
+	    let me = this;
+	    let groups = me.getView().getRootNode().childNodes;
+	    if (me.collapseAllStatus) {
+		groups.forEach(node => node.expand());
+		button.setText(button.defaultText);
+	    } else {
+		button.setText(button.altText);
+		groups.forEach(node => node.collapse());
+	    }
+	    me.collapseAllStatus = !me.collapseAllStatus;
+	},
+
+	checkboxChangeHandler: function(checkbox, filterVMID) {
+	    let me = this;
+	    if (filterVMID) {
+		me.getView().getStore().filter(me.guestFilter());
+	    } else {
+		let filters = me.getView().getStore().getFilters();
+		me.guestFilter().forEach(filter => filters.removeByKey(filter.id));
+	    }
+	},
+    },
+
+    columns: [
+	{
+	    xtype: 'treecolumn',
+	    header: gettext("Backup Group"),
+	    dataIndex: 'text',
+	    renderer: function(value, _metadata, record) {
+		return record.phantom
+		    ? value
+		    : 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,
+	    bind: {
+		hidden: '{!isPBS}',
+	    },
+	},
+	{
+	    header: gettext('Verify State'),
+	    dataIndex: 'verification',
+	    renderer: PVE.Utils.render_backup_verification,
+	    bind: {
+		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,
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('File Restore'),
+	    handler: 'restoreFilesHandler',
+	    bind: {
+		hidden: '{!isPBS}',
+	    },
+	    parentXType: "treepanel",
+	    disabled: true,
+	    enableFn: record => !record.phantom,
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Show Configuration'),
+	    handler: 'showConfigurationHandler',
+	    parentXType: "treepanel",
 	    disabled: true,
-	    selModel: sm,
+	    enableFn: record => !record.phantom,
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Edit Notes'),
+	    handler: 'editNotesHandler',
+	    parentXType: "treepanel",
+	    disabled: true,
+	    enableFn: record => !record.phantom,
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Change Protection'),
+	    handler: 'changeProtectionHandler',
+	    parentXType: "treepanel",
+	    disabled: true,
+	    enableFn: record => !record.phantom,
+	},
+	'-',
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Prune group'),
 	    setBackupGroup: function(backup) {
+		let me = this;
 		if (backup) {
-		    let name = backup.text;
+		    let volid = backup.volid;
 		    let vmid = backup.vmid;
 		    let format = backup.format;
 
-		    let vmtype;
-		    if (name.startsWith('vzdump-lxc-') || format === "pbs-ct") {
-			vmtype = 'lxc';
-		    } else if (name.startsWith('vzdump-qemu-') || format === "pbs-vm") {
-			vmtype = 'qemu';
-		    }
-
+		    let vmtype = PVE.Utils.get_backup_type(volid, format);
 		    if (vmid && vmtype) {
-			this.setText(gettext('Prune group') + ` ${vmtype}/${vmid}`);
-			this.vmid = vmid;
-			this.vmtype = vmtype;
-			this.setDisabled(false);
+			me.setText(gettext('Prune group') + ` ${vmtype}/${vmid}`);
+			me.vmid = vmid;
+			me.vmtype = vmtype;
+			me.setDisabled(false);
 			return;
 		    }
 		}
-		this.setText(gettext('Prune group'));
-		this.vmid = null;
-		this.vmtype = null;
-		this.setDisabled(true);
+		me.setText(gettext('Prune group'));
+		me.vmid = null;
+		me.vmtype = null;
+		me.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,
-		});
-		win.show();
-		win.on('destroy', reload);
+	    handler: 'pruneGroupHandler',
+	    parentXType: "treepanel",
+	    disabled: true,
+	    reference: 'pruneButton',
+	    enableFn: backup => backup.data.groupName !== 'Other',
+	},
+	{
+	    xtype: 'proxmoxButton',
+	    text: gettext('Remove'),
+	    handler: 'removeHandler',
+	    parentXType: 'treepanel',
+	    disabled: true,
+	    enableFn: record => !record.phantom && !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: 'proxmoxAltTextButton',
+	    text: gettext('Collapse all'),
+	    defaultText: gettext('Collapse all'),
+	    altText: gettext('Expand all'),
+	    handler: 'toggleCollapseHandler',
+	    reference: 'collapseToggle',
+	},
+	{
+	    xtype: 'pveStorageSelector',
+	    fieldLabel: gettext('Storage') + ':',
+	    labelAlign: 'right',
+	    labelWidth: 65,
+	    storageContent: 'backup',
+	    reference: 'storagesel',
+	    listeners: {
+		change: 'storageSelectorChange',
+	    },
+	    // hide by default so field doesn't flash when loading the page
+	    hidden: true,
+	    bind: {
+		hidden: '{isStorage}',
+		nodename: '{nodename}',
 	    },
-	});
-
-	me.on('selectionchange', function(model, srecords, eOpts) {
-	    if (srecords.length === 1) {
-		pruneButton.setBackupGroup(srecords[0].data);
-	    } else {
-		pruneButton.setBackupGroup(null);
-	    }
-	});
-
-	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);
-		},
+	},
+	'-',
+	{
+	    xtype: 'checkbox',
+	    boxLabel: gettext('Filter VMID/Type'),
+	    value: 'true',
+	    listeners: {
+		change: 'checkboxChangeHandler',
 	    },
-	];
-	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,
-		    });
-		},
-	    });
-	}
-	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();
-		},
+	    // hide by default so field doesn't flash when loading the page
+	    hidden: true,
+	    bind: {
+		hidden: '{isStorage}',
 	    },
-	    {
-		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}`,
-			layout: 'fit',
-			items: [
-			    {
-				xtype: 'textarea',
-				layout: 'fit',
-				name: 'notes',
-				height: '100%',
-			    },
-			],
-			listeners: {
-			    destroy: () => reload(),
-			},
-		    }).show();
+	},
+	{
+	    xtype: 'textfield',
+	    label: gettext('Search'),
+	    labelAlign: 'right',
+	    width: 200,
+	    enableKeyEvents: true,
+	    emptyText: gettext('Name, Format'),
+	    listeners: {
+		keyup: {
+		    buffer: 500,
+		    fn: 'searchKeyupFn',
 		},
+		change: 'searchChangeFn',
 	    },
-	    {
-		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(),
-		    });
+	    triggers: {
+		clear: {
+		    cls: 'pmx-clear-trigger',
+		    weight: -1,
+		    hidden: true,
+		    handler: 'searchClearHandler',
 		},
 	    },
-	    '-',
-	    pruneButton,
-	);
-
-	if (isPBS) {
-	    me.extraColumns = {
-		encrypted: {
-		    header: gettext('Encrypted'),
-		    dataIndex: 'encrypted',
-		    renderer: PVE.Utils.render_backup_encryption,
-		},
-		verification: {
-		    header: gettext('Verify State'),
-		    dataIndex: 'verification',
-		    renderer: PVE.Utils.render_backup_verification,
-		},
-	    };
-	}
-
-	me.callParent();
+	},
+	],
 
-	me.store.getSorters().clear();
-	me.store.setSorters([
-	    {
-		property: 'vmid',
-		direction: 'ASC',
-	    },
-	    {
-		property: 'vdate',
-		direction: 'DESC',
-	    },
-	]);
+    listeners: {
+	activate: function() {
+	    this.getController().reload();
+	},
+	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 v4 manager 3/4] ui: delete BackupView and replace it with the new Tree BackupView

[Matthias Heiserer]

Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
---
No changes from v2/v3

 www/manager6/Makefile           |   1 -
 www/manager6/grid/BackupView.js | 388 --------------------------------
 www/manager6/lxc/Config.js      |   2 +-
 www/manager6/qemu/Config.js     |   2 +-
 4 files changed, 2 insertions(+), 391 deletions(-)
 delete mode 100644 www/manager6/grid/BackupView.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index e6e01bd1..0575f550 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -73,7 +73,6 @@ JSSRC= 							\
 	form/VNCKeyboardSelector.js			\
 	form/ViewSelector.js				\
 	form/iScsiProviderSelector.js			\
-	grid/BackupView.js				\
 	grid/FirewallAliases.js				\
 	grid/FirewallOptions.js				\
 	grid/FirewallRules.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();
-    },
-});
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 v4 manager 4/4] ui: content view: 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>
---
No changes from v2/v3

 www/manager6/storage/ContentView.js | 43 ++++++-----------------------
 1 file changed, 8 insertions(+), 35 deletions(-)

diff --git a/www/manager6/storage/ContentView.js b/www/manager6/storage/ContentView.js
index 2874b71e..56c11037 100644
--- a/www/manager6/storage/ContentView.js
+++ b/www/manager6/storage/ContentView.js
@@ -147,58 +147,31 @@ Ext.define('PVE.storage.ContentView', {
 	    },
 	);
 
-	let availableColumns = {
-	    'name': {
+	const columns = [
+	    {
 		header: gettext('Name'),
 		flex: 2,
 		sortable: true,
 		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,
 		dataIndex: 'vdate',
 	    },
-	    'format': {
+	    {
 		header: gettext('Format'),
 		width: 100,
 		dataIndex: 'format',
 	    },
-	    'size': {
+	    {
 		header: gettext('Size'),
 		width: 100,
 		renderer: Proxmox.Utils.format_size,
 		dataIndex: 'size',
 	    },
-	};
-
-	let showColumns = me.showColumns || ['name', 'date', 'format', 'size'];
-
-	Object.keys(availableColumns).forEach(function(key) {
-	    if (!showColumns.includes(key)) {
-		delete availableColumns[key];
-	    }
-	});
-
-	if (me.extraColumns && typeof me.extraColumns === 'object') {
-	    Object.assign(availableColumns, me.extraColumns);
-	}
-	const columns = Object.values(availableColumns);
+	];
 
 	Ext.apply(me, {
 	    store: store,
@@ -216,8 +189,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 v4 manager 2/4] ui: storage: Rewrite backup content view as TreePanel.

[Fabian Ebner]

Am 04.04.22 um 15:02 schrieb Matthias Heiserer:
> +Ext.define('PVE.storage.BackupView', {
> +    extend: 'Ext.tree.Panel',
>      alias: 'widget.pveStorageBackupView',
>  
> -    showColumns: ['name', 'notes', 'protected', 'date', 'format', 'size'],
> +    rootVisible: false,
> +    multiColumnSort: true,
> +
> +    title: gettext('Content'),
> +
> +    viewModel: {
> +	data: {
> +	    isPBS: false,
> +	    isStorage: true,

Nit: IMHO fixedStorage might be a little bit more descriptive

> +	},
> +    },
> +

(...)

>  
> -	var sm = me.sm = Ext.create('Ext.selection.RowModel', {});
> +	    let viewModel = me.getViewModel();
> +	    viewModel.set('nodename', me.nodename);
> +	    viewModel.set('isPBS', view.pluginType === 'pbs');
> +
> +	    if (me.vmid) {
> +		me.getView().getStore().filter(me.guestFilter());
> +		viewModel.set('isStorage', false);
> +	    } else {
> +		me.lookup('storagesel').setVisible(false);
> +		me.lookup('backupNowButton').setVisible(false);

Nit: Could have backupNowButton hidden by default and add a binding
(like storagesel already has). Then there should be no need for the else
branch. And having
    viewModel.set('isStorage', !!me.vmid);
outside the if feels a bit cleaner/more future-proof to me.

> +	    }
> +	    Proxmox.Utils.monStoreErrors(view, me.store);
> +	},
> +
> +	onLoad: function(store, records, success, operation) {
> +	    let me = this;
> +	    let view = me.getView();
> +	    let selection = view.getSelection()?.[0];
> +	    selection = selection
> +		? `${selection.parentNode.data.text}${selection.data.volid}`
> +		: false;
> +
> +	    let storage = PVE.data.ResourceStore.findRecord(

Nit: could use the simpler getById()

> +		'id',
> +		`storage/${me.nodename}/${me.storage}`,
> +		0, // startIndex
> +		false, // anyMatch
> +		true, // caseSensitive
> +		true, // exactMatch
> +	    );
> +	    let viewModel = me.getViewModel();
> +	    viewModel.set('isPBS', storage.data.plugintype === 'pbs');
> +
> +	    let expanded = {};
> +	    view.getRootNode().cascadeBy({
> +		before: item => {
> +		    if (item.isExpanded() && !item.data.leaf) {
> +			let id = item.data.text;
> +			expanded[me.storage + '/' + id] = true;
> +			return true;
> +		    }
> +		    return false;
> +		},
> +		after: Ext.emptyFn,
> +	    });
> +	    let groups = me.getRecordGroups(records, expanded);
> +
> +	    for (const item of records.map(i => i.data)) {
> +		item.text = item.volid;
> +		item.leaf = true;
> +		item.iconCls = 'fa-file-o';
> +		item.backupType = PVE.Utils.get_backup_type(item.volid, item.format);
> +		item.groupName = me.getGroupName(item);
> +		groups[me.getGroupName(item)].children.push(item);
> +		groups[me.getGroupName(item)].size += item.size;

Style nit: could create a variable for the group name to avoid calling
the function thrice

> +	    }
> +
> +	    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 (viewModel.get('isPBS')) {
> +		    group.size = latest.size;
> +		}
> +		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

Style nit: same as in v2 ;)

> +		    let id = node.parentNode?.data?.text + node.data?.volid;
> +		    if (id === selection) {
> +			selected = node;
> +			return false;
> +		    }
> +		    return true;
> +		});

(...)

> +
> +	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,

IMHO it looks a bit strange when clicking the 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(),
> +		},
> +	    }).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('searchFilter');
> +	    let volidFilter = Ext.create('Ext.util.Filter', {
> +		property: 'volid',
> +		value: field.getValue(),
> +		anyMatch: true,
> +		caseSensitive: false,
> +	    });
> +	    let formatFilter = Ext.create('Ext.util.Filter', {
> +		property: 'format',
> +		value: field.getValue(),
> +		anyMatch: true,
> +		caseSensitive: false,
> +	    });
> +	    me.getView().getStore().filter({
> +		filterFn: item => volidFilter.filter(item.data) ||
> +		    formatFilter.filter(item.data),
> +		id: 'searchFilter',
> +	    });

Nit: If it's not too much work, it would be nice if searching by the
(displayed) group name would also work on PBS. Or maybe we want to
change the displayed group name to ct/ID vm/ID there (but then one has
to be careful with passing along the correct thing for prune)?

> +	},
> +
> +	searchClearHandler: function(field) {
> +	    field.triggers.clear.setVisible(false);
> +	    field.setValue(this.originalValue);
> +	    this.getView().getStore().getFilters().removeByKey('searchFilter');
> +	},
> +
> +	searchChangeFn: function(field, newValue, oldValue) {
> +	    if (newValue !== field.originalValue) {
> +		field.triggers.clear.setVisible(true);
> +	    }
> +	},
> +
> +	storageSelectorChange: function(self, newValue, oldValue, eOpts) {
> +	    let me = this;
> +	    if (!me.getViewModel().get('isStorage')) {
> +		me.storage = newValue;
> +		me.collapseAllStatus = true;
> +		me.store.on('load', () => me.lookup('collapseToggle').click(), me, { single: true });

Style nit: line too long

> +		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();
> +	},
> +
> +	toggleCollapseHandler: function(button, event) {
> +	    let me = this;
> +	    let groups = me.getView().getRootNode().childNodes;
> +	    if (me.collapseAllStatus) {
> +		groups.forEach(node => node.expand());
> +		button.setText(button.defaultText);
> +	    } else {
> +		button.setText(button.altText);
> +		groups.forEach(node => node.collapse());
> +	    }
> +	    me.collapseAllStatus = !me.collapseAllStatus;
> +	},
> +
> +	checkboxChangeHandler: function(checkbox, filterVMID) {
> +	    let me = this;
> +	    if (filterVMID) {
> +		me.getView().getStore().filter(me.guestFilter());
> +	    } else {
> +		let filters = me.getView().getStore().getFilters();
> +		me.guestFilter().forEach(filter => filters.removeByKey(filter.id));

I feel like we should always filter by backup type in the guest view
like is done currently. Otherwise, there is the possibility to try and
restore e.g. an LXC backup over an existing VM. That probably isn't a
common use case, and it just leads to an error.

(...)

> +	    triggers: {
> +		clear: {
> +		    cls: 'pmx-clear-trigger',
> +		    weight: -1,
> +		    hidden: true,
> +		    handler: 'searchClearHandler',
>  		},
>  	    },
> -	    '-',
> -	    pruneButton,
> -	);
> -
> -	if (isPBS) {
> -	    me.extraColumns = {
> -		encrypted: {
> -		    header: gettext('Encrypted'),
> -		    dataIndex: 'encrypted',
> -		    renderer: PVE.Utils.render_backup_encryption,
> -		},
> -		verification: {
> -		    header: gettext('Verify State'),
> -		    dataIndex: 'verification',
> -		    renderer: PVE.Utils.render_backup_verification,
> -		},
> -	    };
> -	}
> -
> -	me.callParent();
> +	},
> +	],

Style nit: wrong indentation

Re: [pve-devel] [PATCH v4 manager 0/4] BackupView as TreePanel

[Fabian Ebner]

Am 04.04.22 um 15:02 schrieb Matthias Heiserer:
> Depends on https://lists.proxmox.com/pipermail/pve-devel/2022-March/052322.html
> 
> Matthias Heiserer (4):
>   ui: Utils: Helpers for backup type and icon
>   ui: storage: Rewrite backup content view as TreePanel.
>   ui: delete BackupView and replace it with the new Tree BackupView
>   ui: content view: remove dead code
> 
>  www/manager6/Makefile               |   1 -
>  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  | 817 +++++++++++++++++++++-------
>  www/manager6/storage/ContentView.js |  43 +-
>  7 files changed, 657 insertions(+), 616 deletions(-)
>  delete mode 100644 www/manager6/grid/BackupView.js
> 

Great! I've just got one complaint left (and a few nits, see my answer
to 2/4). Repeating the complaint here:

I feel like we should always filter by backup type in the guest view
like is done currently. Otherwise, there is the possibility to try and
restore e.g. an LXC backup over an existing VM. That probably isn't a
common use case, and it just leads to an error.

But even if we don't go for that, consider the series

Reviewed-by: Fabian Ebner <f.ebner@proxmox.com>

Re: [pve-devel] [PATCH v4 manager 0/4] BackupView as TreePanel

[Thomas Lamprecht]

On 06.04.22 13:26, Fabian Ebner wrote:
> Am 04.04.22 um 15:02 schrieb Matthias Heiserer:
>> Depends on https://lists.proxmox.com/pipermail/pve-devel/2022-March/052322.html
>>
>> Matthias Heiserer (4):
>>   ui: Utils: Helpers for backup type and icon
>>   ui: storage: Rewrite backup content view as TreePanel.
>>   ui: delete BackupView and replace it with the new Tree BackupView
>>   ui: content view: remove dead code
>>
>>  www/manager6/Makefile               |   1 -
>>  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  | 817 +++++++++++++++++++++-------
>>  www/manager6/storage/ContentView.js |  43 +-
>>  7 files changed, 657 insertions(+), 616 deletions(-)
>>  delete mode 100644 www/manager6/grid/BackupView.js
>>
> 
> Great! I've just got one complaint left (and a few nits, see my answer
> to 2/4). Repeating the complaint here:
> 
> I feel like we should always filter by backup type in the guest view
> like is done currently. Otherwise, there is the possibility to try and
> restore e.g. an LXC backup over an existing VM. That probably isn't a
> common use case, and it just leads to an error.
> 

Yeah that's a must do IMO.

Re: [pve-devel] [PATCH v4 manager 0/4] BackupView as TreePanel

[Fabian Ebner]

Am 06.04.22 um 17:40 schrieb Thomas Lamprecht:
> On 06.04.22 13:26, Fabian Ebner wrote:
>> Am 04.04.22 um 15:02 schrieb Matthias Heiserer:
>>> Depends on https://lists.proxmox.com/pipermail/pve-devel/2022-March/052322.html
>>>
>>> Matthias Heiserer (4):
>>>   ui: Utils: Helpers for backup type and icon
>>>   ui: storage: Rewrite backup content view as TreePanel.
>>>   ui: delete BackupView and replace it with the new Tree BackupView
>>>   ui: content view: remove dead code
>>>
>>>  www/manager6/Makefile               |   1 -
>>>  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  | 817 +++++++++++++++++++++-------
>>>  www/manager6/storage/ContentView.js |  43 +-
>>>  7 files changed, 657 insertions(+), 616 deletions(-)
>>>  delete mode 100644 www/manager6/grid/BackupView.js
>>>
>>
>> Great! I've just got one complaint left (and a few nits, see my answer
>> to 2/4). Repeating the complaint here:
>>
>> I feel like we should always filter by backup type in the guest view
>> like is done currently. Otherwise, there is the possibility to try and
>> restore e.g. an LXC backup over an existing VM. That probably isn't a
>> common use case, and it just leads to an error.
>>
> 
> Yeah that's a must do IMO.

I might've made it sound worse than it is, by not being specific. By
default, the type *is* filtered. It's just that the checkbox now is
"filter VMID+type" vs. previously, "filter VMID" with type filtering
always active. As turning off that checkbox is a relatively uncommon use
case to begin with, I didn't consider it a must.

Re: [pve-devel] [PATCH v4 manager 0/4] BackupView as TreePanel

[Thomas Lamprecht]

On 07.04.22 08:31, Fabian Ebner wrote:
>>> I feel like we should always filter by backup type in the guest view
>>> like is done currently. Otherwise, there is the possibility to try and
>>> restore e.g. an LXC backup over an existing VM. That probably isn't a
>>> common use case, and it just leads to an error.
>>>
>> Yeah that's a must do IMO.
> I might've made it sound worse than it is, by not being specific. By
> default, the type *is* filtered. It's just that the checkbox now is
> "filter VMID+type" vs. previously, "filter VMID" with type filtering
> always active. As turning off that checkbox is a relatively uncommon use
> case to begin with, I didn't consider it a must.

I still do consider it a must. The checkbox is always shown and easily
accessible, I see no reason to label using it as edge-case and showing backup
files that cannot work can only confuse users and just isn't hard to fix.