[pmg-devel] [PATCH pmg-gui 0/4] low-hanging improvments for the quarantine interface

[pmg-devel] [PATCH pmg-gui 0/4] low-hanging improvments for the quarantine interface

[Stoiko Ivanov]

A few bugzilla entries ask for features in the quarantine interface that
seem quite sensible (e.g. #4137), or are already present in the (by far
more used) spamquarantine, but never made it to the others.

This patchset contains a single small fix, and a refactoring of the
quarantine views to use the same controller.

Would be grateful for feedback

Stoiko Ivanov (4):
  fix #4137: display receiver in attachment/virus quarantine
  quarantine: contextmenu: refactor for use in other quarantines
  quarantine: add common controller for all quarantines
  quarantine: move all quarantines to the new controller

 js/AttachmentQuarantine.js            | 117 +++++------
 js/Makefile                           |   2 +
 js/QuarantineContextMenu.js           |  32 +++
 js/SpamContextMenu.js                 |  14 +-
 js/SpamQuarantine.js                  | 290 +++++++++-----------------
 js/Utils.js                           |  23 +-
 js/VirusQuarantine.js                 |  90 ++------
 js/controller/QuarantineController.js | 170 +++++++++++++++
 8 files changed, 391 insertions(+), 347 deletions(-)
 create mode 100644 js/QuarantineContextMenu.js
 create mode 100644 js/controller/QuarantineController.js

-- 
2.30.2

[pmg-devel] [PATCH pmg-gui 1/4] fix #4137: display receiver in attachment/virus quarantine

[Stoiko Ivanov]

the attachment and virus quarantines contain all quarantined mail for
all recipients - so we should display which mail is being
delivered/deleted - mostly if a mail is sent to multiple addresses
served by the same PMG.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
* the naming of the sender_renderer (which actually just adds the sender
  to the subject it actually renders) was adapted for the function name
  Probably a different choice for both might be clearer
  (sender_subject_renderer, sender_receiver_subject_renderer)?

 js/AttachmentQuarantine.js |  4 ++--
 js/Utils.js                | 23 ++++++++++++++++++-----
 js/VirusQuarantine.js      |  4 ++--
 3 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/js/AttachmentQuarantine.js b/js/AttachmentQuarantine.js
index 6449012..5e41ada 100644
--- a/js/AttachmentQuarantine.js
+++ b/js/AttachmentQuarantine.js
@@ -117,9 +117,9 @@ Ext.define('PMG.AttachmentQuarantine', {
 
 	    columns: [
 		{
-		    header: gettext('Sender/Subject'),
+		    header: `${gettext('Sender')}/${gettext('Receiver')}/${gettext('Subject')}`,
 		    dataIndex: 'subject',
-		    renderer: PMG.Utils.sender_renderer,
+		    renderer: PMG.Utils.sender_receiver_renderer,
 		    flex: 1,
 		},
 		{
diff --git a/js/Utils.js b/js/Utils.js
index 3b54f65..695967d 100644
--- a/js/Utils.js
+++ b/js/Utils.js
@@ -813,15 +813,28 @@ Ext.define('PMG.Utils', {
 	return `<i class='fa ${iconCls}'></i> ${text}`;
     },
 
-    sender_renderer: function(value, metaData, rec) {
-	var subject = Ext.htmlEncode(value);
-	var from = Ext.htmlEncode(rec.data.from);
-	var sender = Ext.htmlEncode(rec.data.sender);
+    addresses_subject_renderer: function(value, metaData, rec, render_receiver) {
+	let subject = Ext.htmlEncode(value);
+	let from = Ext.htmlEncode(rec.data.from);
+	let sender = Ext.htmlEncode(rec.data.sender);
 	if (sender) {
 	    from = Ext.String.format(gettext("{0} on behalf of {1}"),
 				     sender, from);
 	}
-	return '<small>' + from + '</small><br>' + subject;
+	let ret = '<small>' + from;
+	if (render_receiver) {
+	    ret += '<br>To: ' + Ext.htmlEncode(rec.data.receiver);
+	}
+	ret += '</small><br>' + subject;
+	return ret;
+    },
+
+    sender_renderer: function(value, metaData, rec) {
+	return PMG.Utils.addresses_subject_renderer(value, metaData, rec, false);
+    },
+
+    sender_receiver_renderer: function(value, metaData, rec) {
+	return PMG.Utils.addresses_subject_renderer(value, metaData, rec, true);
     },
 
     constructor: function() {
diff --git a/js/VirusQuarantine.js b/js/VirusQuarantine.js
index 7f27188..9e9e3b8 100644
--- a/js/VirusQuarantine.js
+++ b/js/VirusQuarantine.js
@@ -120,9 +120,9 @@ Ext.define('PMG.VirusQuarantine', {
 
 	    columns: [
 		{
-		    header: gettext('Sender/Subject'),
+		    header: `${gettext('Sender')}/${gettext('Receiver')}/${gettext('Subject')}`,
 		    dataIndex: 'subject',
-		    renderer: PMG.Utils.sender_renderer,
+		    renderer: PMG.Utils.sender_receiver_renderer,
 		    flex: 1,
 		},
 		{
-- 
2.30.2

[pmg-devel] [PATCH pmg-gui 2/4] quarantine: contextmenu: refactor for use in other quarantines

[Stoiko Ivanov]

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
can/should probably be squashed with the next patch

 js/Makefile                 |  1 +
 js/QuarantineContextMenu.js | 32 ++++++++++++++++++++++++++++++++
 js/SpamContextMenu.js       | 14 +-------------
 3 files changed, 34 insertions(+), 13 deletions(-)
 create mode 100644 js/QuarantineContextMenu.js

diff --git a/js/Makefile b/js/Makefile
index 0b9f93b..b904598 100644
--- a/js/Makefile
+++ b/js/Makefile
@@ -19,6 +19,7 @@ JSSRC=							\
 	RuleInfo.js					\
 	RuleEditor.js					\
 	MainView.js					\
+	QuarantineContextMenu.js			\
 	QuarantineList.js				\
 	SpamInfoGrid.js					\
 	MailInfo.js					\
diff --git a/js/QuarantineContextMenu.js b/js/QuarantineContextMenu.js
new file mode 100644
index 0000000..4f7744e
--- /dev/null
+++ b/js/QuarantineContextMenu.js
@@ -0,0 +1,32 @@
+Ext.define('PMG.menu.QuarantineContextMenu', {
+    extend: 'Ext.menu.Menu',
+    xtype: 'pmgQuarantineMenu',
+
+    showSeparator: false,
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
+	callCallback: function(btn) {
+	    let view = this.getView();
+	    if (Ext.isFunction(view.callback)) {
+		view.callback(btn.action);
+	    }
+	},
+    },
+
+    items: [
+	{
+	    text: gettext('Deliver'),
+	    iconCls: 'fa fa-fw fa-paper-plane-o',
+	    action: 'deliver',
+	    handler: 'callCallback',
+	},
+	{
+	    text: gettext('Delete'),
+	    iconCls: 'fa fa-fw fa-trash-o',
+	    action: 'delete',
+	    handler: 'callCallback',
+	},
+    ],
+});
+
diff --git a/js/SpamContextMenu.js b/js/SpamContextMenu.js
index 5fb4816..b089f0f 100644
--- a/js/SpamContextMenu.js
+++ b/js/SpamContextMenu.js
@@ -1,17 +1,5 @@
 Ext.define('PMG.menu.SpamContextMenu', {
-    extend: 'Ext.menu.Menu',
-
-    showSeparator: false,
-
-    controller: {
-	xclass: 'Ext.app.ViewController',
-	callCallback: function(btn) {
-	    let view = this.getView();
-	    if (Ext.isFunction(view.callback)) {
-		view.callback(btn.action);
-	    }
-	},
-    },
+    extend: 'PMG.menu.QuarantineContextMenu',
 
     items: [
 	{
-- 
2.30.2

[pmg-devel] [PATCH pmg-gui 3/4] quarantine: add common controller for all quarantines

[Stoiko Ivanov]

over the time the spam quarantine has gained quite a few nice
usability improvments and features, which would be useful in
the virus and attachment quarantines as well - e.g.:

* multi-mail selection
* keyboard actions
* context menu
* download mail as .eml

by adding the common class (which is mostly a copy of the controller
part of SpamQuarantine.js with changes of 'var' to 'let' and removal of the
spam-specific parts), we can reuse it for all the quarantines.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 js/Makefile                           |   1 +
 js/controller/QuarantineController.js | 170 ++++++++++++++++++++++++++
 2 files changed, 171 insertions(+)
 create mode 100644 js/controller/QuarantineController.js

diff --git a/js/Makefile b/js/Makefile
index b904598..9a2bcf2 100644
--- a/js/Makefile
+++ b/js/Makefile
@@ -19,6 +19,7 @@ JSSRC=							\
 	RuleInfo.js					\
 	RuleEditor.js					\
 	MainView.js					\
+	controller/QuarantineController.js		\
 	QuarantineContextMenu.js			\
 	QuarantineList.js				\
 	SpamInfoGrid.js					\
diff --git a/js/controller/QuarantineController.js b/js/controller/QuarantineController.js
new file mode 100644
index 0000000..dfe2915
--- /dev/null
+++ b/js/controller/QuarantineController.js
@@ -0,0 +1,170 @@
+Ext.define('PMG.controller.QuarantineController', {
+    extend: 'Ext.app.ViewController',
+    xtype: 'controller.Quarantine',
+    alias: 'controller.quarantine',
+
+    updatePreview: function(raw, rec) {
+	let preview = this.lookupReference('preview');
+
+	if (!rec || !rec.data || !rec.data.id) {
+	    preview.update('');
+	    preview.setDisabled(true);
+	    return;
+	}
+
+	let url = `/api2/htmlmail/quarantine/content?id=${rec.data.id}`;
+	if (raw) {
+	    url += '&raw=1';
+	}
+	preview.setDisabled(false);
+	this.lookupReference('raw').setDisabled(false);
+	this.lookupReference('download').setDisabled(false);
+	preview.update("<iframe frameborder=0 width=100% height=100% sandbox='allow-same-origin' src='" + url +"'></iframe>");
+    },
+
+    multiSelect: function(selection) {
+	let me = this;
+	me.lookupReference('raw').setDisabled(true);
+	me.lookupReference('download').setDisabled(true);
+	me.lookupReference('mailinfo').setVisible(false);
+
+	let preview = me.lookupReference('preview');
+	preview.setDisabled(false);
+	preview.update(`<h3 style="padding-left:5px;">${gettext('Multiple E-Mails selected')} (${selection.length})</h3>`);
+    },
+
+    toggleRaw: function(button) {
+	let me = this;
+	let list = me.lookupReference('list');
+	let rec = list.selModel.getSelection()[0];
+	me.lookupReference('mailinfo').setVisible(me.raw);
+	me.raw = !me.raw;
+	me.updatePreview(me.raw, rec);
+    },
+
+    btnHandler: function(button, e) {
+	let me = this;
+	let action = button.reference;
+	let list = me.lookupReference('list');
+	let selected = list.getSelection();
+	me.doAction(action, selected);
+    },
+
+    doAction: function(action, selected) {
+	if (!selected.length) {
+	    return;
+	}
+
+	let list = this.lookupReference('list');
+
+	if (selected.length > 1) {
+	    let idlist = selected.map(item => item.data.id);
+	    Ext.Msg.confirm(
+		gettext('Confirm'),
+		Ext.String.format(
+		    gettext("Action '{0}' for '{1}' items"),
+		    action, selected.length,
+		),
+		async function(button) {
+		    if (button !== 'yes') {
+			return;
+		    }
+
+		    list.mask(gettext('Processing...'), 'x-mask-loading');
+
+		    const sliceSize = 2500, maxInFlight = 2;
+		    let batches = [], batchCount = Math.ceil(selected.length / sliceSize);
+		    for (let i = 0; i * sliceSize < selected.length; i++) {
+			let sliceStart = i * sliceSize;
+			let sliceEnd = Math.min(sliceStart + sliceSize, selected.length);
+			batches.push(
+			    PMG.Async.doQAction(
+				action,
+				idlist.slice(sliceStart, sliceEnd),
+				i + 1,
+				batchCount,
+			    ),
+			);
+			if (batches.length >= maxInFlight) {
+			    await Promise.allSettled(batches); // eslint-disable-line no-await-in-loop
+			    batches = [];
+			}
+		    }
+		    await Promise.allSettled(batches); // await possible remaining ones
+		    list.unmask();
+		    // below can be slow, we could remove directly from the in-memory store, but
+		    // with lots of elements and some failures we could be quite out of sync?
+		    list.getController().load();
+		},
+	    );
+	    return;
+	}
+
+	PMG.Utils.doQuarantineAction(action, selected[0].data.id, function() {
+	    let listController = list.getController();
+	    listController.allowPositionSave = false;
+	    // success -> remove directly to avoid slow store reload for a single-element action
+	    list.getStore().remove(selected[0]);
+	    listController.restoreSavedSelection();
+	    listController.allowPositionSave = true;
+	});
+    },
+
+    onSelectMail: function() {
+	let me = this;
+	let list = this.lookupReference('list');
+	let selection = list.selModel.getSelection();
+	if (selection.length > 1) {
+	    me.multiSelect(selection);
+	    return;
+	}
+
+	let rec = selection[0] || {};
+
+	me.getViewModel().set('mailid', rec.data ? rec.data.id : '');
+	me.updatePreview(me.raw || false, rec);
+	me.lookupReference('mailinfo').setVisible(!!rec.data && !me.raw);
+	me.lookupReference('mailinfo').update(rec.data);
+    },
+
+    openContextMenu: function(table, record, tr, index, event) {
+	event.stopEvent();
+	let me = this;
+	let list = me.lookup('list');
+	Ext.create('PMG.menu.QuarantineContextMenu', {
+	    callback: action => me.doAction(action, list.getSelection()),
+	}).showAt(event.getXY());
+    },
+
+    keyPress: function(table, record, item, index, event) {
+	let me = this;
+	let list = me.lookup('list');
+	let key = event.getKey();
+	let action = '';
+	switch (key) {
+	    case event.DELETE:
+	    case 127:
+		action = 'delete';
+		break;
+	    case Ext.event.Event.D:
+	    case Ext.event.Event.D + 32:
+		action = 'deliver';
+		break;
+	}
+
+	if (action !== '') {
+	    me.doAction(action, list.getSelection());
+	}
+    },
+
+    control: {
+	'button[reference=raw]': {
+	    click: 'toggleRaw',
+	},
+	'pmgQuarantineList': {
+	    selectionChange: 'onSelectMail',
+	    itemkeypress: 'keyPress',
+	    rowcontextmenu: 'openContextMenu',
+	},
+    },
+});
-- 
2.30.2

[pmg-devel] [PATCH pmg-gui 4/4] quarantine: move all quarantines to the new controller

[Stoiko Ivanov]

fixes #1674 (the comment about multiselect for the virus quarantine)
fixes #3947 (multiselect for attachment quarantine)

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
* best viewed with `-w`

 js/AttachmentQuarantine.js | 113 +++++++--------
 js/SpamQuarantine.js       | 290 ++++++++++++-------------------------
 js/VirusQuarantine.js      |  86 +++--------
 3 files changed, 164 insertions(+), 325 deletions(-)

diff --git a/js/AttachmentQuarantine.js b/js/AttachmentQuarantine.js
index 5e41ada..831ce0a 100644
--- a/js/AttachmentQuarantine.js
+++ b/js/AttachmentQuarantine.js
@@ -18,87 +18,59 @@ Ext.define('pmg-attachment-list', {
     idProperty: 'id',
 });
 
-Ext.define('PMG.AttachmentQuarantine', {
-    extend: 'Ext.container.Container',
-    xtype: 'pmgAttachmentQuarantine',
-
-    border: false,
-    layout: { type: 'border' },
-
-    defaults: { border: false },
-
-    controller: {
-
-	xclass: 'Ext.app.ViewController',
-
-	updatePreview: function(raw, rec) {
-	    var preview = this.lookupReference('preview');
+Ext.define('PMG.AttachmentQuarantineController', {
+    extend: 'PMG.controller.QuarantineController',
+    alias: 'controller.attachmentquarantine',
+    xtype: 'pmgAttachmentQuarantineController',
+
+    onSelectMail: function() {
+	let me = this;
+	let list = this.lookupReference('list');
+	let selection = list.selModel.getSelection();
+	if (selection.length <= 1) {
+	    let rec = selection[0] || {};
+	    me.lookup('attachmentlist').setID(rec);
+	}
 
-	    if (!rec || !rec.data || !rec.data.id) {
-		preview.update('');
-		preview.setDisabled(true);
-		return;
-	    }
+	me.callParent();
+    },
 
-	    let url = `/api2/htmlmail/quarantine/content?id=${rec.data.id}`;
-	    if (raw) {
-		url += '&raw=1';
-	    }
-	    preview.setDisabled(false);
-	    preview.update("<iframe frameborder=0 width=100% height=100% sandbox='allow-same-origin' src='" + url +"'></iframe>");
+    control: {
+	'button[reference=raw]': {
+	    click: 'toggleRaw',
 	},
-
-	toggleRaw: function(button) {
-	    var me = this;
-	    var list = this.lookupReference('list');
-	    var rec = list.getSelection()[0] || {};
-	    me.lookup('mailinfo').setVisible(me.raw);
-	    me.raw = !me.raw;
-	    me.updatePreview(me.raw, rec);
+	'pmgQuarantineList': {
+	    selectionChange: 'onSelectMail',
 	},
+    },
 
-	btnHandler: function(button, e) {
-	    var list = this.lookupReference('list');
-	    var selected = list.getSelection();
-	    if (!selected.length) {
-		return;
-	    }
+});
 
-	    var action = button.reference;
+Ext.define('PMG.AttachmentQuarantine', {
+    extend: 'Ext.container.Container',
+    xtype: 'pmgAttachmentQuarantine',
 
-	    PMG.Utils.doQuarantineAction(action, selected[0].data.id, function() {
-		list.getController().load();
-	    });
-	},
+    border: false,
+    layout: { type: 'border' },
 
-	onSelectMail: function() {
-	    let me = this;
-	    let list = me.lookup('list');
-	    let rec = list.getSelection()[0] || {};
-	    let mailinfo = me.lookup('mailinfo');
+    defaults: { border: false },
 
-	    me.updatePreview(me.raw || false, rec);
-	    me.lookup('attachmentlist').setID(rec);
-	    mailinfo.setVisible(!!rec.data && !me.raw);
-	    mailinfo.update(rec.data);
+    viewModel: {
+	parent: null,
+	data: {
+	    mailid: '',
 	},
-
-	control: {
-	    'button[reference=raw]': {
-		click: 'toggleRaw',
-	    },
-	    'pmgQuarantineList': {
-		selectionChange: 'onSelectMail',
-	    },
+	formulas: {
+	    downloadMailURL: get => '/api2/json/quarantine/download?mailid=' + encodeURIComponent(get('mailid')),
 	},
-
     },
-
+    controller: 'attachmentquarantine',
     items: [
 	{
 	    title: gettext('Attachment Quarantine'),
 	    xtype: 'pmgQuarantineList',
 	    emptyText: gettext('No data in database'),
+	    selModel: 'checkboxmodel',
 	    emailSelection: false,
 	    reference: 'list',
 	    region: 'west',
@@ -163,6 +135,19 @@ Ext.define('PMG.AttachmentQuarantine', {
 			    iconCls: 'fa fa-file-code-o',
 			},
 			'->',
+			{
+			    xtype: 'button',
+			    reference: 'download',
+			    text: gettext('Download'),
+			    setDownload: function(id) {
+				this.el.dom.download = id + ".eml";
+			    },
+			    bind: {
+				href: '{downloadMailURL}',
+				download: '{mailid}',
+			    },
+			    iconCls: 'fa fa-download',
+			},
 			{
 			    reference: 'deliver',
 			    text: gettext('Deliver'),
diff --git a/js/SpamQuarantine.js b/js/SpamQuarantine.js
index efb7fdc..fdacaec 100644
--- a/js/SpamQuarantine.js
+++ b/js/SpamQuarantine.js
@@ -33,225 +33,123 @@ Ext.define('pmg-spam-list', {
     idProperty: 'id',
 });
 
-Ext.define('PMG.SpamQuarantine', {
-    extend: 'Ext.container.Container',
-    xtype: 'pmgSpamQuarantine',
-
-    border: false,
-    layout: { type: 'border' },
+Ext.define('PMG.SpamQuarantineController', {
+    extend: 'PMG.controller.QuarantineController',
+    xtype: 'pmgSpamQuarantineController',
+    alias: 'controller.spamquarantine',
 
-    defaults: { border: false },
+    updatePreview: function(raw, rec) {
+	let me = this;
+	me.lookupReference('spam').setDisabled(false);
 
-    // from mail link
-    cselect: undefined,
+	me.callParent(arguments);
+    },
 
-    viewModel: {
-	parent: null,
-	data: {
-	    mailid: '',
-	},
-	formulas: {
-	    downloadMailURL: get => '/api2/json/quarantine/download?mailid=' + encodeURIComponent(get('mailid')),
-	},
+    multiSelect: function(selection) {
+	let me = this;
+	let spam = me.lookupReference('spam');
+	spam.setDisabled(true);
+	spam.setPressed(false);
+	me.lookupReference('spaminfo').setVisible(false);
+	me.callParent(selection);
     },
-    controller: {
 
-	xclass: 'Ext.app.ViewController',
+    onSelectMail: function() {
+	let me = this;
+	let list = me.lookupReference('list');
+	let selection = list.selModel.getSelection();
+	if (selection.length <= 1) {
+	    let rec = selection[0] || {};
+	    me.lookupReference('spaminfo').setID(rec);
+	}
+	me.callParent();
+    },
 
-	updatePreview: function(raw, rec) {
-	    var preview = this.lookupReference('preview');
 
-	    if (!rec || !rec.data || !rec.data.id) {
-		preview.update('');
-		preview.setDisabled(true);
-		return;
-	    }
+    toggleSpamInfo: function(btn) {
+	var grid = this.lookupReference('spaminfo');
+	grid.setVisible(!grid.isVisible());
+    },
 
-	    let url = `/api2/htmlmail/quarantine/content?id=${rec.data.id}`;
-	    if (raw) {
-		url += '&raw=1';
-	    }
-	    preview.setDisabled(false);
-	    this.lookupReference('raw').setDisabled(false);
-	    this.lookupReference('spam').setDisabled(false);
-	    this.lookupReference('download').setDisabled(false);
-	    preview.update("<iframe frameborder=0 width=100% height=100% sandbox='allow-same-origin' src='" + url +"'></iframe>");
-	},
+    openContextMenu: function(table, record, tr, index, event) {
+	event.stopEvent();
+	let me = this;
+	let list = me.lookup('list');
+	Ext.create('PMG.menu.SpamContextMenu', {
+	    callback: action => me.doAction(action, list.getSelection()),
+	}).showAt(event.getXY());
+    },
 
-	multiSelect: function(selection) {
-	    var preview = this.lookupReference('preview');
-	    var raw = this.lookupReference('raw');
-	    var spam = this.lookupReference('spam');
-	    var spaminfo = this.lookupReference('spaminfo');
-	    var mailinfo = this.lookupReference('mailinfo');
-	    var download = this.lookupReference('download');
+    keyPress: function(table, record, item, index, event) {
+	var me = this;
+	var list = me.lookup('list');
+	var key = event.getKey();
+	var action = '';
+	switch (key) {
+	    case event.DELETE:
+	    case 127:
+		action = 'delete';
+		break;
+	    case Ext.event.Event.D:
+	    case Ext.event.Event.D + 32:
+		action = 'deliver';
+		break;
+	    case Ext.event.Event.W:
+	    case Ext.event.Event.W + 32:
+		action = 'whitelist';
+		break;
+	    case Ext.event.Event.B:
+	    case Ext.event.Event.B + 32:
+		action = 'blacklist';
+		break;
+	}
 
-	    preview.setDisabled(false);
-	    preview.update(`<h3 style="padding-left:5px;">${gettext('Multiple E-Mails selected')} (${selection.length})</h3>`);
-	    raw.setDisabled(true);
-	    spam.setDisabled(true);
-	    spam.setPressed(false);
-	    spaminfo.setVisible(false);
-	    mailinfo.setVisible(false);
-	    download.setDisabled(true);
-	},
+	if (action !== '') {
+	    me.doAction(action, list.getSelection());
+	}
+    },
 
-	toggleRaw: function(button) {
-	    var me = this;
-	    var list = me.lookupReference('list');
-	    var rec = list.selModel.getSelection()[0];
-	    me.lookupReference('mailinfo').setVisible(me.raw);
-	    me.raw = !me.raw;
-	    me.updatePreview(me.raw, rec);
-	},
+    init: function(view) {
+	this.lookup('list').cselect = view.cselect;
+    },
 
-	btnHandler: function(button, e) {
-	    var me = this;
-	    var action = button.reference;
-	    var list = this.lookupReference('list');
-	    var selected = list.getSelection();
-	    me.doAction(action, selected);
+    control: {
+	'button[reference=raw]': {
+	    click: 'toggleRaw',
 	},
-
-	doAction: function(action, selected) {
-	    if (!selected.length) {
-		return;
-	    }
-
-	    var list = this.lookupReference('list');
-
-	    if (selected.length > 1) {
-		let idlist = selected.map(item => item.data.id);
-		Ext.Msg.confirm(
-		    gettext('Confirm'),
-		    Ext.String.format(
-			gettext("Action '{0}' for '{1}' items"),
-			action, selected.length,
-		    ),
-		    async function(button) {
-			if (button !== 'yes') {
-			    return;
-			}
-
-			list.mask(gettext('Processing...'), 'x-mask-loading');
-
-			const sliceSize = 2500, maxInFlight = 2;
-			let batches = [], batchCount = Math.ceil(selected.length / sliceSize);
-			for (let i = 0; i * sliceSize < selected.length; i++) {
-			    let sliceStart = i * sliceSize;
-			    let sliceEnd = Math.min(sliceStart + sliceSize, selected.length);
-			    batches.push(
-				PMG.Async.doQAction(
-				    action,
-				    idlist.slice(sliceStart, sliceEnd),
-				    i + 1,
-				    batchCount,
-				),
-			    );
-			    if (batches.length >= maxInFlight) {
-				await Promise.allSettled(batches); // eslint-disable-line no-await-in-loop
-				batches = [];
-			    }
-			}
-			await Promise.allSettled(batches); // await possible remaining ones
-			list.unmask();
-			// below can be slow, we could remove directly from the in-memory store, but
-			// with lots of elements and some failures we could be quite out of sync?
-			list.getController().load();
-		    },
-		);
-		return;
-	    }
-
-	    PMG.Utils.doQuarantineAction(action, selected[0].data.id, function() {
-		let listController = list.getController();
-		listController.allowPositionSave = false;
-		// success -> remove directly to avoid slow store reload for a single-element action
-		list.getStore().remove(selected[0]);
-		listController.restoreSavedSelection();
-		listController.allowPositionSave = true;
-	    });
+	'button[reference=spam]': {
+	    click: 'toggleSpamInfo',
 	},
-
-	onSelectMail: function() {
-	    var me = this;
-	    var list = this.lookupReference('list');
-	    var selection = list.selModel.getSelection();
-	    if (selection.length > 1) {
-		me.multiSelect(selection);
-		return;
-	    }
-
-	    var rec = selection[0] || {};
-
-	    me.getViewModel().set('mailid', rec.data ? rec.data.id : '');
-	    me.updatePreview(me.raw || false, rec);
-	    me.lookupReference('spaminfo').setID(rec);
-	    me.lookupReference('mailinfo').setVisible(!!rec.data && !me.raw);
-	    me.lookupReference('mailinfo').update(rec.data);
+	'pmgQuarantineList': {
+	    selectionChange: 'onSelectMail',
+	    itemkeypress: 'keyPress',
+	    rowcontextmenu: 'openContextMenu',
 	},
+    },
+});
 
-	toggleSpamInfo: function(btn) {
-	    var grid = this.lookupReference('spaminfo');
-	    grid.setVisible(!grid.isVisible());
-	},
+Ext.define('PMG.SpamQuarantine', {
+    extend: 'Ext.container.Container',
+    xtype: 'pmgSpamQuarantine',
 
-	openContextMenu: function(table, record, tr, index, event) {
-	    event.stopEvent();
-	    let me = this;
-	    let list = me.lookup('list');
-	    Ext.create('PMG.menu.SpamContextMenu', {
-		callback: action => me.doAction(action, list.getSelection()),
-	    }).showAt(event.getXY());
-	},
+    border: false,
+    layout: { type: 'border' },
 
-	keyPress: function(table, record, item, index, event) {
-	    var me = this;
-	    var list = me.lookup('list');
-	    var key = event.getKey();
-	    var action = '';
-	    switch (key) {
-		case event.DELETE:
-		case 127:
-		    action = 'delete';
-		    break;
-		case Ext.event.Event.D:
-		case Ext.event.Event.D + 32:
-		    action = 'deliver';
-		    break;
-		case Ext.event.Event.W:
-		case Ext.event.Event.W + 32:
-		    action = 'whitelist';
-		    break;
-		case Ext.event.Event.B:
-		case Ext.event.Event.B + 32:
-		    action = 'blacklist';
-		    break;
-	    }
+    defaults: { border: false },
 
-	    if (action !== '') {
-		me.doAction(action, list.getSelection());
-	    }
-	},
+    // from mail link
+    cselect: undefined,
 
-	init: function(view) {
-	    this.lookup('list').cselect = view.cselect;
+    viewModel: {
+	parent: null,
+	data: {
+	    mailid: '',
 	},
-
-	control: {
-	    'button[reference=raw]': {
-		click: 'toggleRaw',
-	    },
-	    'button[reference=spam]': {
-		click: 'toggleSpamInfo',
-	    },
-	    'pmgQuarantineList': {
-		selectionChange: 'onSelectMail',
-		itemkeypress: 'keyPress',
-		rowcontextmenu: 'openContextMenu',
-	    },
+	formulas: {
+	    downloadMailURL: get => '/api2/json/quarantine/download?mailid=' + encodeURIComponent(get('mailid')),
 	},
     },
+    controller: 'spamquarantine',
 
     items: [
 	{
diff --git a/js/VirusQuarantine.js b/js/VirusQuarantine.js
index 9e9e3b8..3c900f9 100644
--- a/js/VirusQuarantine.js
+++ b/js/VirusQuarantine.js
@@ -28,80 +28,23 @@ Ext.define('PMG.VirusQuarantine', {
 
     defaults: { border: false },
 
-    controller: {
-
-	xclass: 'Ext.app.ViewController',
-
-	updatePreview: function(raw) {
-	    var list = this.lookupReference('list');
-	    var rec = list.selModel.getSelection()[0];
-	    var preview = this.lookupReference('preview');
-
-	    if (!rec || !rec.data || !rec.data.id) {
-		preview.update('');
-		preview.setDisabled(true);
-		return;
-	    }
-
-	    let url = `/api2/htmlmail/quarantine/content?id=${rec.data.id}`;
-	    if (raw) {
-		url += '&raw=1';
-	    }
-	    preview.setDisabled(false);
-	    preview.update("<iframe frameborder=0 width=100% height=100% sandbox='allow-same-origin' src='" + url +"'></iframe>");
-	},
-
-	toggleRaw: function(button) {
-	    var me = this;
-	    me.lookup('mailinfo').setVisible(me.raw);
-	    me.raw = !me.raw;
-	    me.updatePreview(me.raw);
+    viewModel: {
+	parent: null,
+	data: {
+	    mailid: '',
 	},
-
-	btnHandler: function(button, e) {
-	    var list = this.lookupReference('list');
-	    var selected = list.getSelection();
-	    if (!selected.length) {
-		return;
-	    }
-
-	    var action = button.reference;
-
-	    PMG.Utils.doQuarantineAction(action, selected[0].data.id, function() {
-		list.getController().load();
-	    });
+	formulas: {
+	    downloadMailURL: get => '/api2/json/quarantine/download?mailid=' + encodeURIComponent(get('mailid')),
 	},
-
-	onSelectMail: function() {
-	    var me = this;
-	    me.updatePreview(me.raw || false);
-	    let mailinfo = me.lookup('mailinfo');
-	    let list = me.lookup('list');
-	    let selection = list.getSelection();
-	    if (selection.length < 1) {
-		mailinfo.setVisible(false);
-		return;
-	    }
-	    mailinfo.setVisible(!me.raw);
-	    mailinfo.update(selection[0].data);
-	},
-
-	control: {
-	    'button[reference=raw]': {
-		click: 'toggleRaw',
-	    },
-	    'pmgQuarantineList': {
-		selectionChange: 'onSelectMail',
-	    },
-	},
-
     },
+    controller: 'quarantine',
 
     items: [
 	{
 	    title: gettext('Virus Quarantine'),
 	    xtype: 'pmgQuarantineList',
 	    emptyText: gettext('No data in database'),
+	    selModel: 'checkboxmodel',
 	    emailSelection: false,
 	    reference: 'list',
 	    region: 'west',
@@ -172,6 +115,19 @@ Ext.define('PMG.VirusQuarantine', {
 			    iconCls: 'fa fa-file-code-o',
 			},
 			'->',
+			{
+			    xtype: 'button',
+			    reference: 'download',
+			    text: gettext('Download'),
+			    setDownload: function(id) {
+				this.el.dom.download = id + ".eml";
+			    },
+			    bind: {
+				href: '{downloadMailURL}',
+				download: '{mailid}',
+			    },
+			    iconCls: 'fa fa-download',
+			},
 			{
 			    reference: 'deliver',
 			    text: gettext('Deliver'),
-- 
2.30.2

Re: [pmg-devel] [PATCH pmg-gui 3/4] quarantine: add common controller for all quarantines

[Thomas Lamprecht]

Am 20/10/2022 um 21:14 schrieb Stoiko Ivanov:
> over the time the spam quarantine has gained quite a few nice
> usability improvments and features, which would be useful in
> the virus and attachment quarantines as well - e.g.:
> 
> * multi-mail selection
> * keyboard actions
> * context menu
> * download mail as .eml
> 
> by adding the common class (which is mostly a copy of the controller
> part of SpamQuarantine.js with changes of 'var' to 'let' and removal of the
> spam-specific parts), we can reuse it for all the quarantines.
> 

great re-use! Can you please move the original implementation in SpamQuarantine
directly over in this path, that'd would better separate the "factor out" and
"add new functionality to components" parts.

> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  js/Makefile                           |   1 +
>  js/controller/QuarantineController.js | 170 ++++++++++++++++++++++++++
>  2 files changed, 171 insertions(+)
>  create mode 100644 js/controller/QuarantineController.js
> 
> diff --git a/js/Makefile b/js/Makefile
> index b904598..9a2bcf2 100644
> --- a/js/Makefile
> +++ b/js/Makefile
> @@ -19,6 +19,7 @@ JSSRC=							\
>  	RuleInfo.js					\
>  	RuleEditor.js					\
>  	MainView.js					\
> +	controller/QuarantineController.js		\
>  	QuarantineContextMenu.js			\
>  	QuarantineList.js				\
>  	SpamInfoGrid.js					\
> diff --git a/js/controller/QuarantineController.js b/js/controller/QuarantineController.js
> new file mode 100644
> index 0000000..dfe2915
> --- /dev/null
> +++ b/js/controller/QuarantineController.js
> @@ -0,0 +1,170 @@
> +Ext.define('PMG.controller.QuarantineController', {

I'd drop the 'Controller' at the end, if one uses the full path to instantiate/refer to this,
it's already clear that it's a controller due to the middle part.

> +    extend: 'Ext.app.ViewController',
> +    xtype: 'controller.Quarantine',

xtype doesn't gets the controller. part, and please keep our general style of prefixing with
the product abbrev, e.g.: 'pmgQuarantine'

> +    alias: 'controller.quarantine',

I guess both as the "wrong" xtype did not work? anyhow, either xtype or alias, but
not both. I've lessend my strong preference of one of them more recently, so I don't
care much which one, more for only using one, not both.

[pmg-devel] applied: [PATCH pmg-gui 1/4] fix #4137: display receiver in attachment/virus quarantine

[Thomas Lamprecht]

Am 20/10/2022 um 21:14 schrieb Stoiko Ivanov:
> the attachment and virus quarantines contain all quarantined mail for
> all recipients - so we should display which mail is being
> delivered/deleted - mostly if a mail is sent to multiple addresses
> served by the same PMG.
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
> * the naming of the sender_renderer (which actually just adds the sender
>   to the subject it actually renders) was adapted for the function name
>   Probably a different choice for both might be clearer
>   (sender_subject_renderer, sender_receiver_subject_renderer)?> 
>  js/AttachmentQuarantine.js |  4 ++--
>  js/Utils.js                | 23 ++++++++++++++++++-----
>  js/VirusQuarantine.js      |  4 ++--
>  3 files changed, 22 insertions(+), 9 deletions(-)
> 
>

applied, thanks! I made a follow up commit cleaning up the code a bit more,
dropping passing along the metaData to the helper fn if it isn't used yet
anyway (if we really get future direct users we can either just wrap it or
re-add it) and transforming the render fns to arrow functions, with render_
in the prefix of the name.

FWIW, I'm not sure that placing the "To" in a new line in the (previously)
"Sender/Subject" column, but maybe rather just add a additional column and
increase the default width of the quarantine grid in attachment/virus Q,
bonus points if it's responsive for smaller (e.g., < 1600px width) windows
/displays, with full hd and bigger the horizontal space is rather unused
anyway. But, that said, it works OK enough as is, and it isn't affecting the
user facing Quarantine, so I don't care to much; just noticed it and wanted
to mention it, maybe someone has a better idea.

[pmg-devel] applied: [PATCH pmg-gui 2/4] quarantine: contextmenu: refactor for use in other quarantines

[Thomas Lamprecht]

Am 20/10/2022 um 21:14 schrieb Stoiko Ivanov:
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
> can/should probably be squashed with the next patch
> 

no, it's better separate.

>  js/Makefile                 |  1 +
>  js/QuarantineContextMenu.js | 32 ++++++++++++++++++++++++++++++++
>  js/SpamContextMenu.js       | 14 +-------------
>  3 files changed, 34 insertions(+), 13 deletions(-)
>  create mode 100644 js/QuarantineContextMenu.js
> 
>

applied, thanks! fwiw, one could only push the white/black list actions
to the item list in an SpamContextMenu's initComponents before calling
the parent, but for just two it really doesn't matters much anyway..