[pbs-devel] [PATCH proxmox-backup 0/5] add context menu to datastore content

[pbs-devel] [PATCH proxmox-backup 0/5] add context menu to datastore content

[Dominik Csapak]

and add a 'show information' window that includes the upload statistics.
This also moves some actions from the column directly into a 'hamburger
menu' so we don't overload the grid.

The only downside to this currently is that i could get keyboard
navigation to work for the context menu or the hamburger menu, but i'm
still looking into this.

This series replaces my old one for the upload statistics [0]


0: https://lists.proxmox.com/pipermail/pbs-devel/2023-August/006343.html

Dominik Csapak (5):
  api-types: add an UploadStatistic api type
  api: datastore admin: add 'snapshot-information' api call
  ui: datastore content: add context menu to groups and snapshots
  ui: datastore content: add snapshot information to context menu
  ui: datastore content: add 'more actions' menu to actions

 pbs-api-types/src/datastore.rs |  36 +++++++
 src/api2/admin/datastore.rs    | 117 +++++++++++++++++++--
 www/Makefile                   |   1 +
 www/datastore/Content.js       | 174 +++++++++++++++++++++++++++----
 www/window/SnapshotInfo.js     | 182 +++++++++++++++++++++++++++++++++
 5 files changed, 486 insertions(+), 24 deletions(-)
 create mode 100644 www/window/SnapshotInfo.js

-- 
2.39.2

[pbs-devel] [PATCH proxmox-backup 1/5] api-types: add an UploadStatistic api type

[Dominik Csapak]

We'll want this to expose on the api, but the internal type used in the
backup environment does not use kebab case, so simply add a new type

I opted for a new type instead of modifying the existing one, so that we
keep the casing for the options in the manifest the same.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 pbs-api-types/src/datastore.rs | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index 1f619c9d..14dc1cc1 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -421,6 +421,22 @@ pub struct SnapshotVerifyState {
     pub state: VerifyState,
 }
 
+#[api()]
+#[derive(Copy, Clone, Serialize, Deserialize, PartialEq)]
+#[serde(rename_all = "kebab-case")]
+/// Chunk upload statistics of a snapshot
+pub struct UploadStatistic {
+    /// Amount of chunks uploaded (incl. duplicates)
+    pub count: u64,
+    /// Uncompressed bytes uploaded
+    pub size: u64,
+    /// Compressed bytes uploaded
+    #[serde(alias = "compressed_size")]
+    pub compressed_size: u64,
+    /// Amount of duplicate chunks uploaded
+    pub duplicates: u64,
+}
+
 /// A namespace provides a logical separation between backup groups from different domains
 /// (cluster, sites, ...) where uniqueness cannot be guaranteed anymore. It allows users to share a
 /// datastore (i.e., one deduplication domain (chunk store)) with multiple (trusted) sites and
-- 
2.39.2

[pbs-devel] [PATCH proxmox-backup 2/5] api: datastore admin: add 'snapshot-information' api call

[Dominik Csapak]

get information about a snapshot. This is intended to contain all
relevant information about a snapshot, so that we don't fill the
'list_snapshots' api call with further info

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
not sure about the naming honestly...

 pbs-api-types/src/datastore.rs |  20 ++++++
 src/api2/admin/datastore.rs    | 117 +++++++++++++++++++++++++++++++--
 2 files changed, 131 insertions(+), 6 deletions(-)

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index 14dc1cc1..54251897 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -421,6 +421,26 @@ pub struct SnapshotVerifyState {
     pub state: VerifyState,
 }
 
+#[api(
+    properties: {
+        info : {
+            type: SnapshotListItem,
+        },
+    },
+)]
+#[derive(Serialize, Deserialize, Clone, PartialEq)]
+#[serde(rename_all = "kebab-case")]
+/// Information about a Snapshot
+pub struct SnapshotInformation {
+    #[serde(flatten)]
+    pub info: SnapshotListItem,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub upload_statistics: Option<UploadStatistic>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    /// "notes" for a backup group
+    pub group_notes: Option<String>,
+}
+
 #[api()]
 #[derive(Copy, Clone, Serialize, Deserialize, PartialEq)]
 #[serde(rename_all = "kebab-case")]
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index a95031e7..759dca81 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -34,12 +34,12 @@ use pxar::EntryKind;
 use pbs_api_types::{
     print_ns_and_snapshot, print_store_and_ns, Authid, BackupContent, BackupNamespace, BackupType,
     Counts, CryptMode, DataStoreListItem, DataStoreStatus, GarbageCollectionStatus, GroupListItem,
-    KeepOptions, Operation, PruneJobOptions, RRDMode, RRDTimeFrame, SnapshotListItem,
-    SnapshotVerifyState, BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA,
-    BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, DATASTORE_SCHEMA, IGNORE_VERIFIED_BACKUPS_SCHEMA,
-    MAX_NAMESPACE_DEPTH, NS_MAX_DEPTH_SCHEMA, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP,
-    PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE, PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY,
-    UPID_SCHEMA, VERIFICATION_OUTDATED_AFTER_SCHEMA,
+    KeepOptions, Operation, PruneJobOptions, RRDMode, RRDTimeFrame, SnapshotInformation,
+    SnapshotListItem, SnapshotVerifyState, UploadStatistic, BACKUP_ARCHIVE_NAME_SCHEMA,
+    BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA, BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA,
+    DATASTORE_SCHEMA, IGNORE_VERIFIED_BACKUPS_SCHEMA, MAX_NAMESPACE_DEPTH, NS_MAX_DEPTH_SCHEMA,
+    PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE,
+    PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, UPID_SCHEMA, VERIFICATION_OUTDATED_AFTER_SCHEMA,
 };
 use pbs_client::pxar::{create_tar, create_zip};
 use pbs_config::CachedUserInfo;
@@ -2239,6 +2239,107 @@ pub async fn set_backup_owner(
     .await?
 }
 
+#[api(
+    input: {
+        properties: {
+            store: { schema: DATASTORE_SCHEMA },
+            ns: {
+                type: BackupNamespace,
+                optional: true,
+            },
+            backup_dir: {
+                type: pbs_api_types::BackupDir,
+                flatten: true,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Anybody,
+        description: "Requires on /datastore/{store}[/{namespace}] either DATASTORE_AUDIT for any \
+            or DATASTORE_BACKUP and being the owner of the group",
+    },
+)]
+/// Get Information about a snapshot
+pub fn get_snapshot_info(
+    store: String,
+    ns: Option<BackupNamespace>,
+    backup_dir: pbs_api_types::BackupDir,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<SnapshotInformation, Error> {
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let ns = ns.unwrap_or_default();
+
+    let datastore = check_privs_and_load_store(
+        &store,
+        &ns,
+        &auth_id,
+        PRIV_DATASTORE_AUDIT,
+        PRIV_DATASTORE_BACKUP,
+        Some(Operation::Read),
+        &backup_dir.group,
+    )?;
+
+    let group = datastore.backup_group(ns.clone(), backup_dir.group.clone());
+    let owner = group.get_owner()?;
+    let backup = datastore.backup_dir(ns.clone(), backup_dir.clone())?;
+
+    let protected = backup.is_protected();
+    let backup_info = BackupInfo::new(backup)?;
+
+    let (manifest, files) = get_all_snapshot_files(&backup_info)?;
+    let comment: Option<String> = manifest.unprotected["notes"].as_str().map(String::from);
+
+    let fingerprint = match manifest.fingerprint() {
+        Ok(fp) => fp,
+        Err(err) => {
+            eprintln!("error parsing fingerprint: '{}'", err);
+            None
+        }
+    };
+
+    let verification = manifest.unprotected["verify_state"].clone();
+    let verification: Option<SnapshotVerifyState> = match serde_json::from_value(verification) {
+        Ok(verify) => verify,
+        Err(err) => {
+            eprintln!("error parsing verification state : '{}'", err);
+            None
+        }
+    };
+
+    let size = Some(files.iter().map(|x| x.size.unwrap_or(0)).sum());
+
+    let group = backup_dir.group.clone();
+
+    let info = SnapshotListItem {
+        backup: backup_dir,
+        comment,
+        verification,
+        fingerprint,
+        files,
+        size,
+        owner: Some(owner),
+        protected,
+    };
+
+    let upload_statistics =
+        match Option::<UploadStatistic>::deserialize(&manifest.unprotected["chunk_upload_stats"]) {
+            Ok(stats) => stats,
+            Err(err) => {
+                log::warn!("error parsing chunk_upload_stats: {err}");
+                None
+            }
+        };
+
+    let note_path = get_group_note_path(&datastore, &ns, &group);
+    let group_notes = file_read_optional_string(note_path)?;
+
+    Ok(SnapshotInformation {
+        info,
+        upload_statistics,
+        group_notes,
+    })
+}
+
 #[sortable]
 const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
     (
@@ -2304,6 +2405,10 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
         &Router::new().download(&API_METHOD_PXAR_FILE_DOWNLOAD),
     ),
     ("rrd", &Router::new().get(&API_METHOD_GET_RRD_STATS)),
+    (
+        "snapshot-information",
+        &Router::new().get(&API_METHOD_GET_SNAPSHOT_INFO),
+    ),
     (
         "snapshots",
         &Router::new()
-- 
2.39.2

[pbs-devel] [PATCH proxmox-backup 3/5] ui: datastore content: add context menu to groups and snapshots

[Dominik Csapak]

and show the relevant actions. They will be forwarded to the controller,
so we can reuse that code without big refactoring them into another
class/place.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
sadly no keyboard navigation in there.. but i'm looking into it

 www/datastore/Content.js | 114 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 114 insertions(+)

diff --git a/www/datastore/Content.js b/www/datastore/Content.js
index 76588a51..8d5d0815 100644
--- a/www/datastore/Content.js
+++ b/www/datastore/Content.js
@@ -870,6 +870,36 @@ Ext.define('PBS.DataStoreContent', {
 		me.firstLoad = true;
 	    }
 	},
+	itemcontextmenu: function(panel, record, item, index, event) {
+	    event.stopEvent();
+	    let menu;
+	    let view = panel.up('pbsDataStoreContent');
+	    let controller = view.getController();
+	    let createControllerCallback = function(name) {
+		return function() {
+		    controller[name](view, undefined, undefined, undefined, undefined, record);
+		};
+	    };
+	    if (record.data.ty === 'group') {
+		menu = Ext.create('PBS.datastore.GroupCmdMenu', {
+		    title: gettext('Group'),
+		    onVerify: createControllerCallback('onVerify'),
+		    onChangeOwner: createControllerCallback('onChangeOwner'),
+		    onPrune: createControllerCallback('onPrune'),
+		    onForget: createControllerCallback('onForget'),
+		});
+	    } else if (record.data.ty === 'dir') {
+		menu = Ext.create('PBS.datastore.SnapshotCmdMenu', {
+		    title: gettext('Snapshot'),
+		    onVerify: createControllerCallback('onVerify'),
+		    onProtectionChange: createControllerCallback('onProtectionChange'),
+		    onForget: createControllerCallback('onForget'),
+		});
+	    }
+	    if (menu) {
+		menu.showAt(event.getXY());
+	    }
+	},
     },
 
     viewConfig: {
@@ -1248,3 +1278,87 @@ Ext.define('PBS.DataStoreContent', {
 	},
     ],
 });
+
+Ext.define('PBS.datastore.GroupCmdMenu', {
+    extend: 'Ext.menu.Menu',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    onVerify: undefined,
+    onChangeOwner: undefined,
+    onPrune: undefined,
+    onForget: undefined,
+
+    items: [
+	{
+	    text: gettext('Verify'),
+	    iconCls: 'pve-icon-verify-lettering',
+	    handler: function() { this.up('menu').onVerify(); },
+	    cbind: {
+		hidden: '{!onVerify}',
+	    },
+	},
+	{
+	    text: gettext('Change owner'),
+	    iconCls: 'fa fa-user',
+	    handler: function() { this.up('menu').onChangeOwner(); },
+	    cbind: {
+		hidden: '{!onChangeOwner}',
+	    },
+	},
+	{
+	    text: gettext('Prune'),
+	    iconCls: 'fa fa-scissors',
+	    handler: function() { this.up('menu').onPrune(); },
+	    cbind: {
+		hidden: '{!onPrune}',
+	    },
+	},
+	{
+	    text: gettext('Remove'),
+	    iconCls: 'fa critical fa-trash-o',
+	    handler: function() { this.up('menu').onForget(); },
+	    cbind: {
+		hidden: '{!onForget}',
+	    },
+	},
+    ],
+});
+
+Ext.define('PBS.datastore.SnapshotCmdMenu', {
+    extend: 'Ext.menu.Menu',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    onVerify: undefined,
+    onProtectionChange: undefined,
+    onForget: undefined,
+
+    items: [
+	{
+	    text: gettext('Verify'),
+	    iconCls: 'pve-icon-verify-lettering',
+	    handler: function() { this.up('menu').onVerify(); },
+	    cbind: {
+		hidden: '{!onVerify}',
+		disabled: '{!onVerify}',
+	    },
+	},
+	{
+	    text: gettext('Change Protection'),
+	    iconCls: 'fa fa-shield',
+	    handler: function() { this.up('menu').onProtectionChange(); },
+	    cbind: {
+		hidden: '{!onProtectionChange}',
+		disabled: '{!onProtectionChange}',
+	    },
+	},
+	{
+	    text: gettext('Remove'),
+	    iconCls: 'fa critical fa-trash-o',
+	    handler: function() { this.up('menu').onForget(); },
+	    cbind: {
+		hidden: '{!onForget}',
+		disabled: '{!onForget}',
+	    },
+	},
+    ],
+});
-- 
2.39.2

[pbs-devel] [PATCH proxmox-backup 4/5] ui: datastore content: add snapshot information to context menu

[Dominik Csapak]

contains a summary of snapshot information, especially interesting since
it contains the upload statistics. This may allow us to reduce the
default column count in the grid.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/Makefile               |   1 +
 www/datastore/Content.js   |  22 +++++
 www/window/SnapshotInfo.js | 182 +++++++++++++++++++++++++++++++++++++
 3 files changed, 205 insertions(+)
 create mode 100644 www/window/SnapshotInfo.js

diff --git a/www/Makefile b/www/Makefile
index 04c12b31..34e0f851 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -86,6 +86,7 @@ JSSRC=							\
 	window/VerifyAll.js				\
 	window/ZFSCreate.js				\
 	window/InfluxDbEdit.js				\
+	window/SnapshotInfo.js				\
 	dashboard/DataStoreStatistics.js		\
 	dashboard/LongestTasks.js			\
 	dashboard/RunningTasks.js			\
diff --git a/www/datastore/Content.js b/www/datastore/Content.js
index 8d5d0815..d9e9d0b4 100644
--- a/www/datastore/Content.js
+++ b/www/datastore/Content.js
@@ -712,6 +712,17 @@ Ext.define('PBS.DataStoreContent', {
 	    });
 	},
 
+	onShowInformation: function(view, rI, cI, item, e, rec) {
+	    let me = this;
+	    Ext.create('PBS.window.SnapshotInfo', {
+		autoShow: true,
+		datastore: view.datastore,
+		'backup-type': rec.data['backup-type'],
+		'backup-id': rec.data['backup-id'],
+		'backup-time': (rec.data["backup-time"].getTime()/1000).toFixed(0),
+	    });
+	},
+
 	onForget: function(table, rI, cI, item, e, { data }) {
 	    let me = this;
 	    let view = this.getView();
@@ -893,6 +904,7 @@ Ext.define('PBS.DataStoreContent', {
 		    title: gettext('Snapshot'),
 		    onVerify: createControllerCallback('onVerify'),
 		    onProtectionChange: createControllerCallback('onProtectionChange'),
+		    onShowInformation: createControllerCallback('onShowInformation'),
 		    onForget: createControllerCallback('onForget'),
 		});
 	    }
@@ -1330,6 +1342,7 @@ Ext.define('PBS.datastore.SnapshotCmdMenu', {
 
     onVerify: undefined,
     onProtectionChange: undefined,
+    onShowInformation: undefined,
     onForget: undefined,
 
     items: [
@@ -1351,6 +1364,15 @@ Ext.define('PBS.datastore.SnapshotCmdMenu', {
 		disabled: '{!onProtectionChange}',
 	    },
 	},
+	{
+	    text: gettext('Show Information'),
+	    iconCls: 'fa fa-info-circle',
+	    handler: function() { this.up('menu').onShowInformation(); },
+	    cbind: {
+		hidden: '{!onShowInformation}',
+		disabled: '{!onShowInformation}',
+	    },
+	},
 	{
 	    text: gettext('Remove'),
 	    iconCls: 'fa critical fa-trash-o',
diff --git a/www/window/SnapshotInfo.js b/www/window/SnapshotInfo.js
new file mode 100644
index 00000000..005f2334
--- /dev/null
+++ b/www/window/SnapshotInfo.js
@@ -0,0 +1,182 @@
+Ext.define('PBS.window.SnapshotInfo', {
+    extend: 'Ext.window.Window',
+    alias: 'widget.pbsSnapshotInfo',
+
+    modal: true,
+    width: 600,
+    resizable: false,
+    referenceHolder: true,
+
+    bodyPadding: 25,
+    defaults: {
+	xtype: 'pmxInfoWidget',
+	printBar: false,
+	padding: '2 0',
+    },
+
+    items: [
+	{
+	    reference: 'backup-time',
+	    iconCls: 'fa fa-clock-o',
+	    title: gettext('Backup Time'),
+	},
+	{
+	    reference: 'protected',
+	    iconCls: 'fa fa-shield',
+	    title: gettext('Protected'),
+	},
+	{
+	    reference: 'size',
+	    iconCls: 'fa fa-hdd-o',
+	    title: gettext('Size'),
+	},
+	{
+	    reference: 'owner',
+	    iconCls: 'fa fa-user-o',
+	    title: gettext('Owner'),
+	},
+	{
+	    // spacer
+	    xtype: 'box',
+	    padding: 10,
+	},
+	{
+	    xtype: 'grid',
+	    reference: 'files',
+	    store: {
+		data: [],
+	    },
+	    height: 170,
+	    scrollable: true,
+	    columns: [
+		{
+		    dataIndex: 'filename',
+		    text: gettext('Filename'),
+		    flex: 1,
+		},
+		{
+		    dataIndex: 'size',
+		    text: gettext('Size'),
+		    flex: 1,
+		    renderer: (v) => v !== undefined ? Proxmox.Utils.format_size(v) : '',
+		},
+		{
+		    dataIndex: 'crypt-mode',
+		    text: gettext('Encrypted'),
+		    flex: 1,
+		    renderer: (v) => {
+			let modeIdx = PBS.Utils.cryptmap[v ?? 'none'] ?? 0;
+			return PBS.Utils.cryptText[modeIdx];
+		    },
+		},
+	    ],
+	},
+	{
+	    // spacer
+	    xtype: 'box',
+	    padding: 10,
+	},
+	{
+	    title: gettext('Verify State'),
+	    iconCls: 'pve-icon-verify-lettering',
+	    reference: 'verify-state',
+	},
+	{
+	    title: gettext('Last Verificaton'),
+	    iconCls: 'fa fa-list-alt',
+	    reference: 'verify-last',
+	},
+	{
+	    // spacer
+	    xtype: 'box',
+	    padding: 10,
+	},
+	{
+	    xtype: 'box',
+	    html: `<i class="fa fa-upload"></i> ${gettext('Upload Statistics')}`,
+	    reference: 'upload-title',
+	    padding: '0 0 10 0',
+	},
+	{
+	    title: gettext('Size'),
+	    reference: 'upload-size',
+	},
+	{
+	    title: gettext('Compressed Size'),
+	    reference: 'compressed-size',
+	},
+	{
+	    title: gettext('Chunk Count'),
+	    reference: 'chunk-count',
+	},
+	{
+	    title: gettext('Duplicate Chunks'),
+	    reference: 'duplicate-chunks',
+	},
+    ],
+
+    initComponent: function() {
+	let me = this;
+	let type = me['backup-type'];
+	let id = me['backup-id'];
+	let time = me['backup-time'];
+	let datetime = new Date(time*1000);
+	if (type === undefined || id === undefined || time === undefined) {
+	    throw "snapshot id not given";
+	}
+
+	let snapshotText = `${type}/${id}/${PBS.Utils.render_datetime_utc(datetime)}`;
+	me.title = Ext.String.format(gettext('Snapshot {0}'), snapshotText);
+
+	me.callParent();
+	Proxmox.Utils.API2Request({
+	    url: `/api2/extjs/admin/datastore/${me.datastore}/snapshot-information`,
+	    params: {
+		'backup-type': type,
+		'backup-id': id,
+		'backup-time': time,
+	    },
+	    method: 'GET',
+	    success: ({ result }) => {
+		me.lookup('backup-time').updateValue(datetime);
+		me.lookup('protected').updateValue(Proxmox.Utils.format_boolean(result.data.protected));
+		me.lookup('size').updateValue(Proxmox.Utils.format_size(result.data.size));
+		me.lookup('owner').updateValue(result.data.owner);
+
+		me.lookup('files').getStore().setData(result.data.files);
+
+		let verification = result.data.verification;
+		if (verification !== undefined) {
+		    let iconCls = 'times critical';
+		    if (verification.state === 'ok') {
+			iconCls = 'check good';
+		    }
+		    let txt = Ext.htmlEncode(verification.state);
+		    let verifyState = `<i class="fa fa-${iconCls}"></i> ${txt}`;
+
+		    let task = Proxmox.Utils.parse_task_upid(verification.upid);
+		    let verifyTime = Proxmox.Utils.render_timestamp(task.starttime);
+		    me.lookup('verify-state').updateValue(verifyState);
+		    me.lookup('verify-last').updateValue(verifyTime);
+		} else {
+		    me.lookup('verify-state').setVisible(false);
+		    me.lookup('verify-last').setVisible(false);
+		}
+
+		let uploadStats = result.data['upload-statistics'];
+		if (uploadStats !== undefined) {
+		    me.lookup('upload-size').updateValue(Proxmox.Utils.format_size(uploadStats.size));
+		    me.lookup('compressed-size').updateValue(Proxmox.Utils.format_size(uploadStats['compressed-size']));
+		    me.lookup('chunk-count').updateValue(uploadStats.count);
+		    me.lookup('duplicate-chunks').updateValue(uploadStats.duplicates);
+		} else {
+		    me.lookup('upload-title').setVisible(false);
+		    me.lookup('upload-size').setVisible(false);
+		    me.lookup('compressed-size').setVisible(false);
+		    me.lookup('chunk-count').setVisible(false);
+		    me.lookup('duplicate-chunks').setVisible(false);
+		}
+	    },
+	});
+    },
+});
-- 
2.39.2

[pbs-devel] [PATCH proxmox-backup 5/5] ui: datastore content: add 'more actions' menu to actions

[Dominik Csapak]

and move change owner and change protection there, since these are not
that often used. Also add the 'Show Information' action there for
snapshots, as this was not available via actions.

Refactor the menu creation for this.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
sadly also here no keyboard ineraction for the menu

 www/datastore/Content.js | 96 ++++++++++++++++++++--------------------
 1 file changed, 49 insertions(+), 47 deletions(-)

diff --git a/www/datastore/Content.js b/www/datastore/Content.js
index d9e9d0b4..981ab300 100644
--- a/www/datastore/Content.js
+++ b/www/datastore/Content.js
@@ -806,6 +806,48 @@ Ext.define('PBS.DataStoreContent', {
 	    }).show();
 	},
 
+	onMoreActions: function(panel, rI, cI, item, event, record) {
+	    this.showContextMenu(panel, record, event, false);
+	},
+
+	showContextMenu: function(panel, record, event, rightClick) {
+	    let me = this;
+	    event.stopEvent();
+	    let menu;
+	    let view = me.getView();
+	    let createControllerCallback = function(name) {
+		return function() {
+		    me[name](view, undefined, undefined, undefined, undefined, record);
+		};
+	    };
+	    if (record.data.ty === 'group') {
+		let params = {
+		    title: gettext('Group'),
+		    onChangeOwner: createControllerCallback('onChangeOwner'),
+		};
+		if (rightClick) {
+		    params.onVerify = createControllerCallback('onVerify');
+		    params.onPrune = createControllerCallback('onPrune');
+		    params.onForget = createControllerCallback('onForget');
+		}
+		menu = Ext.create('PBS.datastore.GroupCmdMenu', params);
+	    } else if (record.data.ty === 'dir') {
+		let params = {
+		    title: gettext('Snapshot'),
+		    onProtectionChange: createControllerCallback('onProtectionChange'),
+		    onShowInformation: createControllerCallback('onShowInformation'),
+		};
+		if (rightClick) {
+		    params.onVerify = createControllerCallback('onVerify');
+		    params.onForget = createControllerCallback('onForget');
+		}
+		menu = Ext.create('PBS.datastore.SnapshotCmdMenu', params);
+	    }
+	    if (menu) {
+		menu.showAt(event.getXY());
+	    }
+	},
+
 	filter: function(item, value) {
 	    if (item.data.text.indexOf(value) !== -1) {
 		return true;
@@ -882,35 +924,7 @@ Ext.define('PBS.DataStoreContent', {
 	    }
 	},
 	itemcontextmenu: function(panel, record, item, index, event) {
-	    event.stopEvent();
-	    let menu;
-	    let view = panel.up('pbsDataStoreContent');
-	    let controller = view.getController();
-	    let createControllerCallback = function(name) {
-		return function() {
-		    controller[name](view, undefined, undefined, undefined, undefined, record);
-		};
-	    };
-	    if (record.data.ty === 'group') {
-		menu = Ext.create('PBS.datastore.GroupCmdMenu', {
-		    title: gettext('Group'),
-		    onVerify: createControllerCallback('onVerify'),
-		    onChangeOwner: createControllerCallback('onChangeOwner'),
-		    onPrune: createControllerCallback('onPrune'),
-		    onForget: createControllerCallback('onForget'),
-		});
-	    } else if (record.data.ty === 'dir') {
-		menu = Ext.create('PBS.datastore.SnapshotCmdMenu', {
-		    title: gettext('Snapshot'),
-		    onVerify: createControllerCallback('onVerify'),
-		    onProtectionChange: createControllerCallback('onProtectionChange'),
-		    onShowInformation: createControllerCallback('onShowInformation'),
-		    onForget: createControllerCallback('onForget'),
-		});
-	    }
-	    if (menu) {
-		menu.showAt(event.getXY());
-	    }
+	    this.getController().showContextMenu(panel, record, event, true);
 	},
     },
 
@@ -1000,30 +1014,12 @@ Ext.define('PBS.DataStoreContent', {
 		        ? 'pve-icon-verify-lettering' : 'pmx-hidden',
 		    isActionDisabled: (v, r, c, i, rec) => !!rec.data.leaf,
                 },
-                {
-		    handler: 'onChangeOwner',
-		    getClass: (v, m, { data }) => data.ty === 'group' ? 'fa fa-user' : 'pmx-hidden',
-		    getTip: (v, m, rec) => Ext.String.format(gettext("Change owner of '{0}'"), v),
-		    isActionDisabled: (v, r, c, i, { data }) => data.ty !== 'group',
-                },
 		{
 		    handler: 'onPrune',
 		    getTip: (v, m, rec) => Ext.String.format(gettext("Prune '{0}'"), v),
 		    getClass: (v, m, { data }) => data.ty === 'group' ? 'fa fa-scissors' : 'pmx-hidden',
 		    isActionDisabled: (v, r, c, i, { data }) => data.ty !== 'group',
 		},
-		{
-		    handler: 'onProtectionChange',
-		    getTip: (v, m, rec) => Ext.String.format(gettext("Change protection of '{0}'"), v),
-		    getClass: (v, m, rec) => {
-			if (rec.data.ty === 'dir') {
-			    let extraCls = rec.data.protected ? 'good' : 'faded';
-			    return `fa fa-shield ${extraCls}`;
-			}
-			return 'pmx-hidden';
-		    },
-		    isActionDisabled: (v, r, c, i, rec) => rec.data.ty !== 'dir',
-		},
 		{
 		    handler: 'onForget',
 		    getTip: (v, m, { data }) => {
@@ -1044,6 +1040,12 @@ Ext.define('PBS.DataStoreContent', {
 		        : 'pmx-hidden',
 		    isActionDisabled: (v, r, c, i, { data }) => false,
 		},
+		{
+		    handler: 'onMoreActions',
+		    getTip: (v, m, { data }) => gettext('Show more actions'),
+		    getClass: (v, m, { data }) => data.ty === 'group' || data.ty === 'dir' ? 'fa fa-bars' : 'pmx-hidden',
+		    isActionDisabled: (v, r, c, i, { data }) => data.ty !== 'group' && data.ty !== 'dir',
+		},
 		{
 		    handler: 'downloadFile',
 		    getTip: (v, m, rec) => Ext.String.format(gettext("Download '{0}'"), v),
-- 
2.39.2

Re: [pbs-devel] [PATCH proxmox-backup 0/5] add context menu to datastore content

[Fabian Grünbichler]

On November 28, 2023 10:43 am, Dominik Csapak wrote:
> and add a 'show information' window that includes the upload statistics.
> This also moves some actions from the column directly into a 'hamburger
> menu' so we don't overload the grid.
> 
> The only downside to this currently is that i could get keyboard
> navigation to work for the context menu or the hamburger menu, but i'm
> still looking into this.
> 
> This series replaces my old one for the upload statistics [0]

Tested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>

seems to work as expected

> 0: https://lists.proxmox.com/pipermail/pbs-devel/2023-August/006343.html
> 
> Dominik Csapak (5):
>   api-types: add an UploadStatistic api type
>   api: datastore admin: add 'snapshot-information' api call
>   ui: datastore content: add context menu to groups and snapshots
>   ui: datastore content: add snapshot information to context menu
>   ui: datastore content: add 'more actions' menu to actions
> 
>  pbs-api-types/src/datastore.rs |  36 +++++++
>  src/api2/admin/datastore.rs    | 117 +++++++++++++++++++--
>  www/Makefile                   |   1 +
>  www/datastore/Content.js       | 174 +++++++++++++++++++++++++++----
>  www/window/SnapshotInfo.js     | 182 +++++++++++++++++++++++++++++++++
>  5 files changed, 486 insertions(+), 24 deletions(-)
>  create mode 100644 www/window/SnapshotInfo.js
> 
> -- 
> 2.39.2
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
> 

Re: [pbs-devel] [PATCH proxmox-backup 0/5] add context menu to datastore content

[Philipp Hufnagl]

LGTM


Tested-by: Philipp Hufnagl <p.hufnagl@proxmox.com>


On 11/28/23 10:43, Dominik Csapak wrote:
> and add a 'show information' window that includes the upload statistics.
> This also moves some actions from the column directly into a 'hamburger
> menu' so we don't overload the grid.
> 
> The only downside to this currently is that i could get keyboard
> navigation to work for the context menu or the hamburger menu, but i'm
> still looking into this.
> 
> This series replaces my old one for the upload statistics [0]
> 
> 
> 0: https://lists.proxmox.com/pipermail/pbs-devel/2023-August/006343.html
> 
> Dominik Csapak (5):
>   api-types: add an UploadStatistic api type
>   api: datastore admin: add 'snapshot-information' api call
>   ui: datastore content: add context menu to groups and snapshots
>   ui: datastore content: add snapshot information to context menu
>   ui: datastore content: add 'more actions' menu to actions
> 
>  pbs-api-types/src/datastore.rs |  36 +++++++
>  src/api2/admin/datastore.rs    | 117 +++++++++++++++++++--
>  www/Makefile                   |   1 +
>  www/datastore/Content.js       | 174 +++++++++++++++++++++++++++----
>  www/window/SnapshotInfo.js     | 182 +++++++++++++++++++++++++++++++++
>  5 files changed, 486 insertions(+), 24 deletions(-)
>  create mode 100644 www/window/SnapshotInfo.js
> 

Re: [pbs-devel] [PATCH proxmox-backup 0/5] add context menu to datastore content

[Max Carrara]

On 11/28/23 10:43, Dominik Csapak wrote:
> and add a 'show information' window that includes the upload statistics.
> This also moves some actions from the column directly into a 'hamburger
> menu' so we don't overload the grid.
> 
> The only downside to this currently is that i could get keyboard
> navigation to work for the context menu or the hamburger menu, but i'm
> still looking into this.
> 
> This series replaces my old one for the upload statistics [0]

Tested it as well - looks pretty good! Works as described.

The code also seems rather straightforward; I cannot comment too much on
the JS side of things, but all in all there's nothing I could complain
about.

Tested-by: Max Carrara <m.carrara@proxmox.com>
Reviewed-by: Max Carrara <m.carrara@proxmox.com>

> 
> 
> 0: https://lists.proxmox.com/pipermail/pbs-devel/2023-August/006343.html
> 
> Dominik Csapak (5):
>   api-types: add an UploadStatistic api type
>   api: datastore admin: add 'snapshot-information' api call
>   ui: datastore content: add context menu to groups and snapshots
>   ui: datastore content: add snapshot information to context menu
>   ui: datastore content: add 'more actions' menu to actions
> 
>  pbs-api-types/src/datastore.rs |  36 +++++++
>  src/api2/admin/datastore.rs    | 117 +++++++++++++++++++--
>  www/Makefile                   |   1 +
>  www/datastore/Content.js       | 174 +++++++++++++++++++++++++++----
>  www/window/SnapshotInfo.js     | 182 +++++++++++++++++++++++++++++++++
>  5 files changed, 486 insertions(+), 24 deletions(-)
>  create mode 100644 www/window/SnapshotInfo.js
> 

[pbs-devel] applied: [PATCH proxmox-backup 3/5] ui: datastore content: add context menu to groups and snapshots

[Thomas Lamprecht]

Am 28/11/2023 um 10:44 schrieb Dominik Csapak:
> and show the relevant actions. They will be forwarded to the controller,
> so we can reuse that code without big refactoring them into another
> class/place.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> sadly no keyboard navigation in there.. but i'm looking into it
> 
>  www/datastore/Content.js | 114 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 114 insertions(+)
> 
>

applied this one for now, thanks!

Albeit, I'm wondering if we should add the ID into the menu header, for
snapshots that tends to be a bit large though...

Oh, and fwiw, we could add a "Copy Text" for the cells where it makes
sense (ID of group or snapshot, comment firt line, maybe date) so that
one has it slightly easier if they found the wanted snapshot/group here
and what to use that ID for, e.g., finding that in the PVE storage
content UI, but no hard feelings, just as an idea.

Re: [pbs-devel] applied: [PATCH proxmox-backup 3/5] ui: datastore content: add context menu to groups and snapshots

[Dominik Csapak]

On 11/28/23 16:27, Thomas Lamprecht wrote:
> Am 28/11/2023 um 10:44 schrieb Dominik Csapak:
>> and show the relevant actions. They will be forwarded to the controller,
>> so we can reuse that code without big refactoring them into another
>> class/place.
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>> sadly no keyboard navigation in there.. but i'm looking into it
>>
>>   www/datastore/Content.js | 114 +++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 114 insertions(+)
>>
>>
> 
> applied this one for now, thanks!
> 
> Albeit, I'm wondering if we should add the ID into the menu header, for
> snapshots that tends to be a bit large though...

yeah i tried that, but couldn't make it look ok and consistent, so i left it out.
If you have an idea how to format it (maybe "type \n id \n time" ?)
it should be relatively trivial to implement.

> 
> Oh, and fwiw, we could add a "Copy Text" for the cells where it makes
> sense (ID of group or snapshot, comment firt line, maybe date) so that
> one has it slightly easier if they found the wanted snapshot/group here
> and what to use that ID for, e.g., finding that in the PVE storage
> content UI, but no hard feelings, just as an idea.

yeah that could make sense, or maybe easier if we allow selecting the
text of the cells directly? that way the users can select copy themselves.
(we'd have to show the context menu depending on if text is selected or
not of course, but that should be doable without much issues)

Re: [pbs-devel] applied: [PATCH proxmox-backup 3/5] ui: datastore content: add context menu to groups and snapshots

[Thomas Lamprecht]

Am 29/11/2023 um 08:35 schrieb Dominik Csapak:
> On 11/28/23 16:27, Thomas Lamprecht wrote:
>> Albeit, I'm wondering if we should add the ID into the menu header, for
>> snapshots that tends to be a bit large though...
> 
> yeah i tried that, but couldn't make it look ok and consistent, so i left it out.
> If you have an idea how to format it (maybe "type \n id \n time" ?)
> it should be relatively trivial to implement.
> 

No, besides making the menu just wider by default (some padding is totally
fine to have) and make text-overflow ellipsis to avoid breakage (which is
not 100% ideal either) I have no real good idea.

>>
>> Oh, and fwiw, we could add a "Copy Text" for the cells where it makes
>> sense (ID of group or snapshot, comment firt line, maybe date) so that
>> one has it slightly easier if they found the wanted snapshot/group here
>> and what to use that ID for, e.g., finding that in the PVE storage
>> content UI, but no hard feelings, just as an idea.
> 
> yeah that could make sense, or maybe easier if we allow selecting the
> text of the cells directly? that way the users can select copy themselves.
> (we'd have to show the context menu depending on if text is selected or
> not of course, but that should be doable without much issues)
> 

Not so sure about that, at least because double-click is already used
for expand/collapse or note edit, so at least my default workflow of
double click text so select word-ranges doesn't really works, but that
might be relatively minor personal use case (fwiw, I run into that often
in the Notes boxes we have for guests in PVE).

Maybe let's skip that for now maybe we get some real user demand from
which we can better judge the UX that makes most sense here.