[pmg-devel] [PATCH pmg-api/pmg-gui v2 0/2] fix #3450: batch deletion/delivery for postfix queue

[pmg-devel] [PATCH pmg-api/pmg-gui v2 0/2] fix #3450: batch deletion/delivery for postfix queue

[Hannes Laimer]

Adds a new POST endpoint that accepts both a type(delete/deliver) and a
list of id's. This is also how batch operations are done for quarantine.

https://bugzilla.proxmox.com/show_bug.cgi?id=3450

v2:
 - instead of 'filter' use the IDs directly
 - UI now doesn't have extra buttons, the existing Remove and Flush ones
   will just work with either one or multiple selected items

pmg-api:

Hannes Laimer (1):
  fix #3450: api: queue: add POST endpoint for batch deletion/delivery

 src/PMG/API2/Postfix.pm | 43 +++++++++++++++++++++++++++++++++++++++++
 src/PMG/Postfix.pm      | 33 +++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+)


pmg-gui:

Hannes Laimer (1):
  fix #3450: ui: queue: multi-select for item deletion/delivery

 js/PostfixMailQueue.js | 104 +++++++++++++++++++++++++++--------------
 1 file changed, 68 insertions(+), 36 deletions(-)


Summary over all repositories:
  3 files changed, 144 insertions(+), 36 deletions(-)

-- 
Generated by git-murpp 0.8.1


_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] [PATCH pmg-api v2 1/1] fix #3450: api: queue: add POST endpoint for batch deletion/delivery

[Hannes Laimer]

Delivery is done using `postqueue -i <id>`, this is slower than using
`postsuper -r -`. But `postsuper -r -` would only re-queue the mails,
they'd also recieve new IDs. Flush should rather be an immediate delivery
attempt.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
for 2000 mails flush took ~8s, since 2000 is the max we can do at once
through the UI I think this is ok. Alternatively we could also disable
the Flush button if more than 200 or so a selected, but I don't think
there is much value in that.

v2:
- single POST endpoitn that takes type(delete/deliver) and list of id's

 src/PMG/API2/Postfix.pm | 43 +++++++++++++++++++++++++++++++++++++++++
 src/PMG/Postfix.pm      | 33 +++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+)

diff --git a/src/PMG/API2/Postfix.pm b/src/PMG/API2/Postfix.pm
index ba0689c..90a74e9 100644
--- a/src/PMG/API2/Postfix.pm
+++ b/src/PMG/API2/Postfix.pm
@@ -335,6 +335,49 @@ __PACKAGE__->register_method({
     },
 });
 
+__PACKAGE__->register_method({
+    name => 'queue_action',
+    path => 'queue/{queue}',
+    method => 'POST',
+    description => "Perform an action on the given queue IDs (delete/deliver).",
+    proxyto => 'node',
+    permissions => { check => ['admin'] },
+    protected => 1,
+    parameters => {
+        additionalProperties => 0,
+        properties => {
+            node => get_standard_option('pve-node'),
+            queue => $queue_name_option,
+            action => {
+                description => 'Operation to perform on the given IDs.',
+                type => 'string',
+                enum => ['delete', 'deliver'],
+            },
+            id => {
+                description => 'Queue ID(s), separated by semicolons (;).',
+                type => 'string',
+                pattern => '[A-Za-z0-9]{8,20}(;[A-Za-z0-9]{8,20})*',
+            },
+        },
+    },
+    returns => { type => 'null' },
+    code => sub {
+        my ($param) = @_;
+
+        my @ids = defined($param->{id}) && length($param->{id})
+            ? split(/\s*;\s*/, $param->{id})
+            : ();
+
+        if ($param->{action} eq 'delete') {
+            PMG::Postfix::delete_queue_ids($param->{queue}, \@ids);
+        } elsif ($param->{action} eq 'deliver') {
+            PMG::Postfix::flush_queue_ids(\@ids);
+        }
+
+        return undef;
+    },
+});
+
 __PACKAGE__->register_method({
     name => 'delete_queue',
     path => 'queue/{queue}',
diff --git a/src/PMG/Postfix.pm b/src/PMG/Postfix.pm
index 966130f..dff8ec6 100644
--- a/src/PMG/Postfix.pm
+++ b/src/PMG/Postfix.pm
@@ -221,6 +221,39 @@ sub delete_queue {
     PVE::Tools::run_command($cmd);
 }
 
+# delete multiple mails by queue IDs
+sub delete_queue_ids {
+    my ($queue, $ids) = @_;
+
+    return if !$ids || ref($ids) ne 'ARRAY' || !@$ids;
+
+    my %seen;
+    my @queue_ids;
+    foreach my $qid (@$ids) {
+        next if !$qid;
+        next if $seen{$qid}++;
+        push @queue_ids, $qid;
+    }
+
+    return if !@queue_ids;
+
+    my $input = join("\n", @queue_ids) . "\n";
+    my $cmd = ['/usr/sbin/postsuper', '-d', '-'];
+    push @$cmd, $queue if defined($queue);
+    PVE::Tools::run_command($cmd, input => $input);
+}
+
+# flush for multiple queue IDs
+sub flush_queue_ids {
+    my ($ids) = @_;
+
+    return if !$ids || ref($ids) ne 'ARRAY' || !@$ids;
+
+    foreach my $qid (@$ids) {
+        PVE::Tools::run_command(['/usr/sbin/postqueue', '-i', $qid]);
+    }
+}
+
 sub discard_verify_cache {
     unlink "/var/lib/postfix/verify_cache.db";
 
-- 
2.47.3



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

Re: [pmg-devel] [PATCH pmg-api v2 1/1] fix #3450: api: queue: add POST endpoint for batch deletion/delivery

[Thomas Lamprecht]

Am 23.09.25 um 11:33 schrieb Hannes Laimer:
> Delivery is done using `postqueue -i <id>`, this is slower than using
> `postsuper -r -`. But `postsuper -r -` would only re-queue the mails,
> they'd also recieve new IDs. Flush should rather be an immediate delivery
> attempt.
> 
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> for 2000 mails flush took ~8s, since 2000 is the max we can do at once
> through the UI I think this is ok. Alternatively we could also disable
> the Flush button if more than 200 or so a selected, but I don't think
> there is much value in that.

Such info is fine to have recorded in the commit message

> 
> v2:
> - single POST endpoitn that takes type(delete/deliver) and list of id's

Fine for delivery, but why not add a new optional ids parameter to the
existing DELETE endpoint?

I mean, I have no strong objection against this, but I'd at least like
to have a reason recorded in the commit message for why that route was
not chosen.

> 
>  src/PMG/API2/Postfix.pm | 43 +++++++++++++++++++++++++++++++++++++++++
>  src/PMG/Postfix.pm      | 33 +++++++++++++++++++++++++++++++
>  2 files changed, 76 insertions(+)
> 
> diff --git a/src/PMG/API2/Postfix.pm b/src/PMG/API2/Postfix.pm
> index ba0689c..90a74e9 100644
> --- a/src/PMG/API2/Postfix.pm
> +++ b/src/PMG/API2/Postfix.pm
> @@ -335,6 +335,49 @@ __PACKAGE__->register_method({
>      },
>  });
>  
> +__PACKAGE__->register_method({
> +    name => 'queue_action',
> +    path => 'queue/{queue}',
> +    method => 'POST',
> +    description => "Perform an action on the given queue IDs (delete/deliver).",
> +    proxyto => 'node',
> +    permissions => { check => ['admin'] },
> +    protected => 1,
> +    parameters => {
> +        additionalProperties => 0,
> +        properties => {
> +            node => get_standard_option('pve-node'),
> +            queue => $queue_name_option,
> +            action => {
> +                description => 'Operation to perform on the given IDs.',
> +                type => 'string',
> +                enum => ['delete', 'deliver'],
> +            },
> +            id => {

"ids" would slightly better signal that this can be more than one.

> +                description => 'Queue ID(s), separated by semicolons (;).',
> +                type => 'string',
> +                pattern => '[A-Za-z0-9]{8,20}(;[A-Za-z0-9]{8,20})*',

hmm, don't we have a format for this or at least an existing regex
we can reuse?

> +            },
> +        },
> +    },
> +    returns => { type => 'null' },
> +    code => sub {
> +        my ($param) = @_;
> +
> +        my @ids = defined($param->{id}) && length($param->{id})
> +            ? split(/\s*;\s*/, $param->{id})
> +            : ();

I'd prefer using PVE::Tools' split_list method, which handles most edge
cases already.

> +
> +        if ($param->{action} eq 'delete') {
> +            PMG::Postfix::delete_queue_ids($param->{queue}, \@ids);
> +        } elsif ($param->{action} eq 'deliver') {
> +            PMG::Postfix::flush_queue_ids(\@ids);
> +        }
> +
> +        return undef;
> +    },
> +});
> +
>  __PACKAGE__->register_method({
>      name => 'delete_queue',
>      path => 'queue/{queue}',
> diff --git a/src/PMG/Postfix.pm b/src/PMG/Postfix.pm
> index 966130f..dff8ec6 100644
> --- a/src/PMG/Postfix.pm
> +++ b/src/PMG/Postfix.pm
> @@ -221,6 +221,39 @@ sub delete_queue {
>      PVE::Tools::run_command($cmd);
>  }
>  
> +# delete multiple mails by queue IDs
> +sub delete_queue_ids {
> +    my ($queue, $ids) = @_;
> +
> +    return if !$ids || ref($ids) ne 'ARRAY' || !@$ids;
> +
> +    my %seen;
> +    my @queue_ids;
> +    foreach my $qid (@$ids) {

please prefer for over foreach for new code.

> +        next if !$qid;
> +        next if $seen{$qid}++;
> +        push @queue_ids, $qid;
> +    }
> +
> +    return if !@queue_ids;
> +
> +    my $input = join("\n", @queue_ids) . "\n";

I mean, if you already iterate above, why not assemble the input string
directly there? While sometimes it can be nicer for code readability to
use a intermediate array This is short enough to still be easy enough to
read and grasp.

And btw. as of now you have the $id 4 times in memory:

1. in $ids
2. in %seen (well deduplicated, but still)
3. in @queue_ids
4. in $input

If you want to cope with duplicates then it would be probably even better
to just do

my $unique_qids = { map { $_ => 1 } $ids->@* };

my $input = join("\n", keys $unique_qids->%*) . "\n";

That avoids at least one copy.

> +    my $cmd = ['/usr/sbin/postsuper', '-d', '-'];
> +    push @$cmd, $queue if defined($queue);
> +    PVE::Tools::run_command($cmd, input => $input);
> +}
> +
> +# flush for multiple queue IDs
> +sub flush_queue_ids {
> +    my ($ids) = @_;
> +
> +    return if !$ids || ref($ids) ne 'ARRAY' || !@$ids;
> +
> +    foreach my $qid (@$ids) {

s/foreach/for/

> +        PVE::Tools::run_command(['/usr/sbin/postqueue', '-i', $qid]);
> +    }
> +}
> +
>  sub discard_verify_cache {
>      unlink "/var/lib/postfix/verify_cache.db";
>  



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

[pmg-devel] [PATCH pmg-gui v2 1/1] fix #3450: ui: queue: multi-select for item deletion/delivery

[Hannes Laimer]

ExtJS does not allow having a "select all" checkbox in the header for
buffered stores, that is why we have to use `Ext.data.Store` instead.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
filtering did not feel as instant as it did with the buffered store, but
not really noticable

 js/PostfixMailQueue.js | 104 +++++++++++++++++++++++++++--------------
 1 file changed, 68 insertions(+), 36 deletions(-)

diff --git a/js/PostfixMailQueue.js b/js/PostfixMailQueue.js
index 17c7d05..7fd690d 100644
--- a/js/PostfixMailQueue.js
+++ b/js/PostfixMailQueue.js
@@ -22,8 +22,14 @@ Ext.define('PMG.Postfix.MailQueue', {
 
     queuename: 'deferred',
 
+    selModel: {
+        selType: 'checkboxmodel',
+        mode: 'MULTI',
+        showHeaderCheckbox: true,
+    },
+
     store: {
-        xclass: 'Ext.data.BufferedStore',
+        xclass: 'Ext.data.Store',
         model: 'pmg-mailq',
         remoteFilter: true,
         remoteSort: true,
@@ -54,44 +60,62 @@ Ext.define('PMG.Postfix.MailQueue', {
 
         onFlush: function (button, event, rec) {
             var view = this.getView();
-
-            Proxmox.Utils.API2Request({
-                url:
-                    '/api2/extjs/nodes/' +
-                    view.nodename +
-                    '/postfix/queue/' +
-                    view.queuename +
-                    '/' +
-                    rec.data.queue_id,
-                method: 'POST',
-                waitMsgTarget: view,
-                failure: function (response, opts) {
-                    Ext.Msg.alert(gettext('Error'), response.htmlStatus);
-                },
-            });
+            let sel = view.getSelectionModel().getSelection();
+            if (sel && sel.length > 0) {
+                let ids = sel.map((r) => r.get('queue_id'));
+                Ext.Msg.show({
+                    title: gettext('Confirm'),
+                    message: Ext.String.format(gettext("Deliver {0} selected mails now?"), ids.length),
+                    buttons: Ext.Msg.YESNO,
+                    icon: Ext.Msg.WARNING,
+                    fn: function (btn) {
+                        if (btn !== 'yes') { return; }
+                        Proxmox.Utils.API2Request({
+                            url:
+                                '/api2/extjs/nodes/' +
+                                view.nodename +
+                                '/postfix/queue/' +
+                                view.queuename,
+                            method: 'POST',
+                            params: { action: 'deliver', id: ids.join(';') },
+                            waitMsgTarget: view,
+                            success: function () { view.selModel.deselectAll(); view.store.load(); },
+                            failure: function (response) { Ext.Msg.alert(gettext('Error'), response.htmlStatus); },
+                        });
+                    },
+                });
+                return;
+            }
         },
 
         onRemove: function (button, event, rec) {
             var view = this.getView();
-
-            Proxmox.Utils.API2Request({
-                url:
-                    '/api2/extjs/nodes/' +
-                    view.nodename +
-                    '/postfix/queue/' +
-                    view.queuename +
-                    '/' +
-                    rec.data.queue_id,
-                method: 'DELETE',
-                waitMsgTarget: view,
-                success: function (response, opts) {
-                    view.selModel.deselectAll();
-                    view.store.load();
-                },
-                failure: function (response, opts) {
-                    Ext.Msg.alert(gettext('Error'), response.htmlStatus);
-                },
-            });
+            let sel = view.getSelectionModel().getSelection();
+            if (sel && sel.length > 0) {
+                let ids = sel.map((r) => r.get('queue_id'));
+                Ext.Msg.show({
+                    title: gettext('Confirm'),
+                    message: Ext.String.format(gettext("Delete {0} selected mails?"), ids.length),
+                    buttons: Ext.Msg.YESNO,
+                    icon: Ext.Msg.WARNING,
+                    fn: function (btn) {
+                        if (btn !== 'yes') { return; }
+                        Proxmox.Utils.API2Request({
+                            url:
+                                '/api2/extjs/nodes/' +
+                                view.nodename +
+                                '/postfix/queue/' +
+                                view.queuename,
+                            method: 'POST',
+                            params: { action: 'delete', id: ids.join(';') },
+                            waitMsgTarget: view,
+                            success: function () { view.selModel.deselectAll(); view.store.load(); },
+                            failure: function (response) { Ext.Msg.alert(gettext('Error'), response.htmlStatus); },
+                        });
+                    },
+                });
+                return;
+            }
         },
 
         onHeaders: function (button, event, rec) {
@@ -130,6 +154,12 @@ Ext.define('PMG.Postfix.MailQueue', {
     tbar: [
         {
             xtype: 'proxmoxButton',
+            reference: 'headerBtn',
+            enableFn: function (rec) {
+                let grid = this.up('grid');
+                if (!grid) { return false; }
+                return grid.getSelectionModel().getCount() === 1;
+            },
             disabled: true,
             text: gettext('Headers'),
             handler: 'onHeaders',
@@ -141,7 +171,9 @@ Ext.define('PMG.Postfix.MailQueue', {
             handler: 'onFlush',
         },
         {
-            xtype: 'proxmoxStdRemoveButton',
+            xtype: 'proxmoxButton',
+            disabled: true,
+            text: gettext('Remove'),
             handler: 'onRemove',
         },
         {
-- 
2.47.3



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel

Re: [pmg-devel] [PATCH pmg-gui v2 1/1] fix #3450: ui: queue: multi-select for item deletion/delivery

[Thomas Lamprecht]

Am 23.09.25 um 11:34 schrieb Hannes Laimer:
> ExtJS does not allow having a "select all" checkbox in the header for
> buffered stores, that is why we have to use `Ext.data.Store` instead.
> 
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> filtering did not feel as instant as it did with the buffered store, but
> not really noticable
> 
>  js/PostfixMailQueue.js | 104 +++++++++++++++++++++++++++--------------
>  1 file changed, 68 insertions(+), 36 deletions(-)
> 
> diff --git a/js/PostfixMailQueue.js b/js/PostfixMailQueue.js
> index 17c7d05..7fd690d 100644
> --- a/js/PostfixMailQueue.js
> +++ b/js/PostfixMailQueue.js
> @@ -22,8 +22,14 @@ Ext.define('PMG.Postfix.MailQueue', {
>  
>      queuename: 'deferred',
>  
> +    selModel: {
> +        selType: 'checkboxmodel',
> +        mode: 'MULTI',
> +        showHeaderCheckbox: true,
> +    },
> +
>      store: {
> -        xclass: 'Ext.data.BufferedStore',
> +        xclass: 'Ext.data.Store',
>          model: 'pmg-mailq',
>          remoteFilter: true,
>          remoteSort: true,
> @@ -54,44 +60,62 @@ Ext.define('PMG.Postfix.MailQueue', {
>  
>          onFlush: function (button, event, rec) {
>              var view = this.getView();
> -
> -            Proxmox.Utils.API2Request({
> -                url:
> -                    '/api2/extjs/nodes/' +
> -                    view.nodename +
> -                    '/postfix/queue/' +
> -                    view.queuename +
> -                    '/' +
> -                    rec.data.queue_id,
> -                method: 'POST',
> -                waitMsgTarget: view,
> -                failure: function (response, opts) {
> -                    Ext.Msg.alert(gettext('Error'), response.htmlStatus);
> -                },
> -            });
> +            let sel = view.getSelectionModel().getSelection();
> +            if (sel && sel.length > 0) {
> +                let ids = sel.map((r) => r.get('queue_id'));
> +                Ext.Msg.show({
> +                    title: gettext('Confirm'),
> +                    message: Ext.String.format(gettext("Deliver {0} selected mails now?"), ids.length),
> +                    buttons: Ext.Msg.YESNO,
> +                    icon: Ext.Msg.WARNING,

Is this really something we want to use a warning for?

Also, will it now prompt for a single mail too? Would like to avoid doing
that for the single mail edge case.

> +                    fn: function (btn) {
> +                        if (btn !== 'yes') { return; }
> +                        Proxmox.Utils.API2Request({
> +                            url:
> +                                '/api2/extjs/nodes/' +
> +                                view.nodename +
> +                                '/postfix/queue/' +
> +                                view.queuename,

could use a template string `/api2/extjs/nodes/${view.nodename}/postfix/queue/${view.queuename}`

> +                            method: 'POST',
> +                            params: { action: 'deliver', id: ids.join(';') },
> +                            waitMsgTarget: view,
> +                            success: function () { view.selModel.deselectAll(); view.store.load(); },

would rather have above function split over multiple lines.

> +                            failure: function (response) { Ext.Msg.alert(gettext('Error'), response.htmlStatus); },

can be written as:

failure: (response) => Ext.Msg.alert(gettext('Error'), response.htmlStatus),

> +                        });
> +                    },
> +                });
> +                return;
> +            }
>          },
>  
>          onRemove: function (button, event, rec) {
>              var view = this.getView();
> -
> -            Proxmox.Utils.API2Request({
> -                url:
> -                    '/api2/extjs/nodes/' +
> -                    view.nodename +
> -                    '/postfix/queue/' +
> -                    view.queuename +
> -                    '/' +
> -                    rec.data.queue_id,
> -                method: 'DELETE',
> -                waitMsgTarget: view,
> -                success: function (response, opts) {
> -                    view.selModel.deselectAll();
> -                    view.store.load();
> -                },
> -                failure: function (response, opts) {
> -                    Ext.Msg.alert(gettext('Error'), response.htmlStatus);
> -                },
> -            });

Most comments from above also apply to the code below.

> +            let sel = view.getSelectionModel().getSelection();
> +            if (sel && sel.length > 0) {
> +                let ids = sel.map((r) => r.get('queue_id'));
> +                Ext.Msg.show({
> +                    title: gettext('Confirm'),
> +                    message: Ext.String.format(gettext("Delete {0} selected mails?"), ids.length),
> +                    buttons: Ext.Msg.YESNO,
> +                    icon: Ext.Msg.WARNING,
> +                    fn: function (btn) {
> +                        if (btn !== 'yes') { return; }
> +                        Proxmox.Utils.API2Request({
> +                            url:
> +                                '/api2/extjs/nodes/' +
> +                                view.nodename +
> +                                '/postfix/queue/' +
> +                                view.queuename,
> +                            method: 'POST',
> +                            params: { action: 'delete', id: ids.join(';') },
> +                            waitMsgTarget: view,
> +                            success: function () { view.selModel.deselectAll(); view.store.load(); },
> +                            failure: function (response) { Ext.Msg.alert(gettext('Error'), response.htmlStatus); },
> +                        });
> +                    },
> +                });
> +                return;
> +            }
>          },
>  
>          onHeaders: function (button, event, rec) {
> @@ -130,6 +154,12 @@ Ext.define('PMG.Postfix.MailQueue', {
>      tbar: [
>          {
>              xtype: 'proxmoxButton',
> +            reference: 'headerBtn',
> +            enableFn: function (rec) {
> +                let grid = this.up('grid');
> +                if (!grid) { return false; }
> +                return grid.getSelectionModel().getCount() === 1;
> +            },
>              disabled: true,
>              text: gettext('Headers'),
>              handler: 'onHeaders',
> @@ -141,7 +171,9 @@ Ext.define('PMG.Postfix.MailQueue', {
>              handler: 'onFlush',
>          },
>          {
> -            xtype: 'proxmoxStdRemoveButton',
> +            xtype: 'proxmoxButton',
> +            disabled: true,
> +            text: gettext('Remove'),
>              handler: 'onRemove',
>          },
>          {



_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel