[pve-devel] [PATCH manager] ui: backup job overview: add filter field

[pve-devel] [PATCH manager] ui: backup job overview: add filter field

[Dominik Csapak]

so that users can easily search their jobs for comments, VMID and pool
names, in case there are many backup jobs.

This partially addresses #5721, since one can search for vmid when they
are selected directly, but not when inside a pool. Still should be a
useful addition for users with many backup jobs.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/dc/Backup.js | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 381402ca..a96eb5bf 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -780,6 +780,45 @@ Ext.define('PVE.dc.BackupView', {
 		    '-',
 		    run_btn,
 		    '->',
+		    {
+			xtype: 'textfield',
+			fieldLabel: gettext('Filter'),
+			autoEl: {
+			    tag: 'div',
+			    'data-qtip': gettext('Filters by Comment, VMID or Pool name'),
+			},
+			triggers: {
+			    clear: {
+				cls: 'pmx-clear-trigger',
+				weight: -1,
+				hidden: true,
+				handler: function() {
+				    this.setValue('');
+				    this.getTriggers().clear.setVisible(false);
+				},
+			    },
+			},
+			labelAlign: 'right',
+			listeners: {
+			    change: {
+				fn: function(search, val) {
+				    search.getTriggers().clear.setVisible(!!val);
+				    store.clearFilter();
+				    store.filterBy((record) => {
+					let found = false;
+					for (const field of ['comment', 'vmid', 'pool']) {
+					    if (record.data[field]?.toString().includes(val)) {
+						found = true;
+						break;
+					    }
+					}
+					return found;
+				    });
+				},
+				buffer: 250,
+			    },
+			},
+		    },
 		    noBackupJobInfoButton,
 		    '-',
 		    {
-- 
2.39.2



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

Re: [pve-devel] [PATCH manager] ui: backup job overview: add filter field

[Aaron Lauterer]

gave this a quick test spin and if I clear my search, the other backup 
jobs that did not match don't show up anymore. So resetting seems to be 
somewhat broken.


On a more general level, the question is, if this approach will be 
sufficient or if we don't want something in the direction of "show me 
all backup jobs that cover this specific VMID". Which will need a bit 
more logic. Then the question is, do we want to handle that in the 
backend, where we already have it, or do we handle it purely in the 
frontend where we will need to replicate the logic.

For example, all VMIDs on a node. all VMIDs except the selected. VMIDs 
in resource pools, ...

On  2024-09-19  16:30, Dominik Csapak wrote:
> so that users can easily search their jobs for comments, VMID and pool
> names, in case there are many backup jobs.
> 
> This partially addresses #5721, since one can search for vmid when they
> are selected directly, but not when inside a pool. Still should be a
> useful addition for users with many backup jobs.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>   www/manager6/dc/Backup.js | 39 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 39 insertions(+)
> 
> diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
> index 381402ca..a96eb5bf 100644
> --- a/www/manager6/dc/Backup.js
> +++ b/www/manager6/dc/Backup.js
> @@ -780,6 +780,45 @@ Ext.define('PVE.dc.BackupView', {
>   		    '-',
>   		    run_btn,
>   		    '->',
> +		    {
> +			xtype: 'textfield',
> +			fieldLabel: gettext('Filter'),
> +			autoEl: {
> +			    tag: 'div',
> +			    'data-qtip': gettext('Filters by Comment, VMID or Pool name'),
> +			},
> +			triggers: {
> +			    clear: {
> +				cls: 'pmx-clear-trigger',
> +				weight: -1,
> +				hidden: true,
> +				handler: function() {
> +				    this.setValue('');
> +				    this.getTriggers().clear.setVisible(false);
> +				},
> +			    },
> +			},
> +			labelAlign: 'right',
> +			listeners: {
> +			    change: {
> +				fn: function(search, val) {
> +				    search.getTriggers().clear.setVisible(!!val);
> +				    store.clearFilter();
> +				    store.filterBy((record) => {
> +					let found = false;
> +					for (const field of ['comment', 'vmid', 'pool']) {
> +					    if (record.data[field]?.toString().includes(val)) {
> +						found = true;
> +						break;
> +					    }
> +					}
> +					return found;
> +				    });
> +				},
> +				buffer: 250,
> +			    },
> +			},
> +		    },
>   		    noBackupJobInfoButton,
>   		    '-',
>   		    {



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

Re: [pve-devel] [PATCH manager] ui: backup job overview: add filter field

[Dominik Csapak]

On 9/19/24 17:36, Aaron Lauterer wrote:
> gave this a quick test spin and if I clear my search, the other backup jobs that did not match don't 
> show up anymore. So resetting seems to be somewhat broken.

interesting, tested here with chrome and firefox and it works fine.
what exactly did you press/put in/etc. ?

> 
> 
> On a more general level, the question is, if this approach will be sufficient or if we don't want 
> something in the direction of "show me all backup jobs that cover this specific VMID". Which will 
> need a bit more logic. Then the question is, do we want to handle that in the backend, where we 
> already have it, or do we handle it purely in the frontend where we will need to replicate the logic.
> 
> For example, all VMIDs on a node. all VMIDs except the selected. VMIDs in resource pools, ...

Fair, as that would "really" fix the bug. Just for the record, I did decide
for a 'front-end' only approach because,

* less api calls/parameters to maintain
* it probably solves most use cases (and searching inside pools/etc. could also be done here)

If we'd want to have a more comprehensive vmid <-> backup job lookup,
i'd probably put it into the VM backup panel itself, or maybe as an addition to this filter
as an explicit way to search for a vmid

no hard feelings either way though :)

> 
> On  2024-09-19  16:30, Dominik Csapak wrote:
>> so that users can easily search their jobs for comments, VMID and pool
>> names, in case there are many backup jobs.
>>
>> This partially addresses #5721, since one can search for vmid when they
>> are selected directly, but not when inside a pool. Still should be a
>> useful addition for users with many backup jobs.
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>>   www/manager6/dc/Backup.js | 39 +++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 39 insertions(+)
>>
>> diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
>> index 381402ca..a96eb5bf 100644
>> --- a/www/manager6/dc/Backup.js
>> +++ b/www/manager6/dc/Backup.js
>> @@ -780,6 +780,45 @@ Ext.define('PVE.dc.BackupView', {
>>               '-',
>>               run_btn,
>>               '->',
>> +            {
>> +            xtype: 'textfield',
>> +            fieldLabel: gettext('Filter'),
>> +            autoEl: {
>> +                tag: 'div',
>> +                'data-qtip': gettext('Filters by Comment, VMID or Pool name'),
>> +            },
>> +            triggers: {
>> +                clear: {
>> +                cls: 'pmx-clear-trigger',
>> +                weight: -1,
>> +                hidden: true,
>> +                handler: function() {
>> +                    this.setValue('');
>> +                    this.getTriggers().clear.setVisible(false);
>> +                },
>> +                },
>> +            },
>> +            labelAlign: 'right',
>> +            listeners: {
>> +                change: {
>> +                fn: function(search, val) {
>> +                    search.getTriggers().clear.setVisible(!!val);
>> +                    store.clearFilter();
>> +                    store.filterBy((record) => {
>> +                    let found = false;
>> +                    for (const field of ['comment', 'vmid', 'pool']) {
>> +                        if (record.data[field]?.toString().includes(val)) {
>> +                        found = true;
>> +                        break;
>> +                        }
>> +                    }
>> +                    return found;
>> +                    });
>> +                },
>> +                buffer: 250,
>> +                },
>> +            },
>> +            },
>>               noBackupJobInfoButton,
>>               '-',
>>               {
> 



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

Re: [pve-devel] [PATCH manager] ui: backup job overview: add filter field

[Aaron Lauterer]

after a quick debugging session we realized that if a backup job has no 
selection, we need to handle that.

This is not a normal situation and I most likely had these after 
deleting the VMs and removing them from the job config.

Inline where the fix can be placed:

On  2024-09-20  08:09, Dominik Csapak wrote:
> On 9/19/24 17:36, Aaron Lauterer wrote:
>> gave this a quick test spin and if I clear my search, the other backup 
>> jobs that did not match don't show up anymore. So resetting seems to 
>> be somewhat broken.
> 
> interesting, tested here with chrome and firefox and it works fine.
> what exactly did you press/put in/etc. ?
> 
>>
>>
>> On a more general level, the question is, if this approach will be 
>> sufficient or if we don't want something in the direction of "show me 
>> all backup jobs that cover this specific VMID". Which will need a bit 
>> more logic. Then the question is, do we want to handle that in the 
>> backend, where we already have it, or do we handle it purely in the 
>> frontend where we will need to replicate the logic.
>>
>> For example, all VMIDs on a node. all VMIDs except the selected. VMIDs 
>> in resource pools, ...
> 
> Fair, as that would "really" fix the bug. Just for the record, I did decide
> for a 'front-end' only approach because,
> 
> * less api calls/parameters to maintain
> * it probably solves most use cases (and searching inside pools/etc. 
> could also be done here)
> 
> If we'd want to have a more comprehensive vmid <-> backup job lookup,
> i'd probably put it into the VM backup panel itself, or maybe as an 
> addition to this filter
> as an explicit way to search for a vmid
> 
> no hard feelings either way though :)
> 
>>
>> On  2024-09-19  16:30, Dominik Csapak wrote:
>>> so that users can easily search their jobs for comments, VMID and pool
>>> names, in case there are many backup jobs.
>>>
>>> This partially addresses #5721, since one can search for vmid when they
>>> are selected directly, but not when inside a pool. Still should be a
>>> useful addition for users with many backup jobs.
>>>
>>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>>> ---
>>>   www/manager6/dc/Backup.js | 39 +++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 39 insertions(+)
>>>
>>> diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
>>> index 381402ca..a96eb5bf 100644
>>> --- a/www/manager6/dc/Backup.js
>>> +++ b/www/manager6/dc/Backup.js
>>> @@ -780,6 +780,45 @@ Ext.define('PVE.dc.BackupView', {
>>>               '-',
>>>               run_btn,
>>>               '->',
>>> +            {
>>> +            xtype: 'textfield',
>>> +            fieldLabel: gettext('Filter'),
>>> +            autoEl: {
>>> +                tag: 'div',
>>> +                'data-qtip': gettext('Filters by Comment, VMID or 
>>> Pool name'),
>>> +            },
>>> +            triggers: {
>>> +                clear: {
>>> +                cls: 'pmx-clear-trigger',
>>> +                weight: -1,
>>> +                hidden: true,
>>> +                handler: function() {
>>> +                    this.setValue('');
>>> +                    this.getTriggers().clear.setVisible(false);
>>> +                },
>>> +                },
>>> +            },
>>> +            labelAlign: 'right',
>>> +            listeners: {
>>> +                change: {
>>> +                fn: function(search, val) {
>>> +                    search.getTriggers().clear.setVisible(!!val);
>>> +                    store.clearFilter();

here a

if (!val) {
	return;
}

will fix the issue
>>> +                    store.filterBy((record) => {
>>> +                    let found = false;
>>> +                    for (const field of ['comment', 'vmid', 'pool']) {
>>> +                        if 
>>> (record.data[field]?.toString().includes(val)) {
>>> +                        found = true;
>>> +                        break;
>>> +                        }
>>> +                    }
>>> +                    return found;
>>> +                    });
>>> +                },
>>> +                buffer: 250,
>>> +                },
>>> +            },
>>> +            },
>>>               noBackupJobInfoButton,
>>>               '-',
>>>               {
>>
> 
> 



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

Re: [pve-devel] [PATCH manager] ui: backup job overview: add filter field

[Thomas Lamprecht]

Am 20/09/2024 um 08:09 schrieb Dominik Csapak:
>> For example, all VMIDs on a node. all VMIDs except the selected. VMIDs in resource pools, ...
> Fair, as that would "really" fix the bug. Just for the record, I did decide
> for a 'front-end' only approach because,
> 
> * less api calls/parameters to maintain
> * it probably solves most use cases (and searching inside pools/etc. could also be done here)
> 
> If we'd want to have a more comprehensive vmid <-> backup job lookup,
> i'd probably put it into the VM backup panel itself, or maybe as an addition to this filter
> as an explicit way to search for a vmid
> 
> no hard feelings either way though 🙂

FWIW, with the general job system it could be also an option to make a
generic job filter API where each job plugin can implement a method to
decide what/how to search for.

That way we could also create a "show backup jobs that guest is included
in" at the virtual guest's backup panel, or even a notice that it isn't
included in any (could be also done as a separate guest "health check"
window to avoid the cost of querying this every time the guest is visited
in the UI).

Anyhow, as while I understand why you went for this approach, it surely is
simpler and quicker to implement, I think that users could be confused by
not seeing a job for a VMID because it's not explicitly listed.
Especially once we get a tag-based selection, which would be great to have
soonish, I think that manual VMID selection will become less popular that
it is now already, as it's easier to re-use existing groupings compared to
having to update jobs on every guest creation/destruction.

One possibility could be to add this without VMIDs for now, albeit that's
naturally not ideal as the prime use case is checking in which job, if at
all, a VMID is included.

In summary, I think it's better to go for a "complete" solution here, even
if it means that we have to add some new API endpoints, if an evaluation
shows that the generic job-filter API is feasible, adding that endpoint
might give us a quite good ROI there though. If it doesn't seems to be
easily possible we still could go for a purely frontend based solution.


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